Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WFCORE-7114] Deprecate use of ModuleIdentifier in ModuleDefinition #6302

Merged
merged 1 commit into from
Dec 23, 2024
Merged

[WFCORE-7114] Deprecate use of ModuleIdentifier in ModuleDefinition #6302

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions server/src/main/java/org/jboss/as/server/moduleservice/ModuleDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,31 @@ public class ModuleDefinition {
private final Set<ModuleDependency> dependencies;
private final ModuleSpec moduleSpec;


/** @deprecated use {@link #ModuleDefinition(String, Set, ModuleSpec)} */
@Deprecated(forRemoval = true)
public ModuleDefinition(final ModuleIdentifier moduleIdentifier, final Set<ModuleDependency> dependencies, final ModuleSpec moduleSpec) {
this.moduleIdentifier = moduleIdentifier;
this.dependencies = dependencies;
this.moduleSpec = moduleSpec;
}

public ModuleDefinition(final String moduleIdentifier, final Set<ModuleDependency> dependencies, final ModuleSpec moduleSpec) {
this.moduleIdentifier = ModuleIdentifier.fromString(moduleIdentifier); // inefficient but this is unused. When we switch the use to this we'll store the string.
this.dependencies = dependencies;
this.moduleSpec = moduleSpec;
}

/** @deprecated use {@link #getModuleName()} */
@Deprecated(forRemoval = true)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a rule of thumb, we are always asking for the since attribute on all @deprecated annotations so we can make an idea without looking at the Git history when it was deprecated.

This can be done later though

public ModuleIdentifier getModuleIdentifier() {
return moduleIdentifier;
}

public String getModuleName() {
return moduleIdentifier.toString(); // inefficient but this is unused. When we switch the use to this we'll store the string.
}
Comment on lines +49 to +51
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if getModuleName is what we would want. The ModuleIdentifier is composed by name and slot attributes. So getModuleName() returning the result of moduleIdentifier.toString() maybe is not what we want since it could drive into confusion since someone could be expecting to get just the ModuleIdentifier name.

As additional detail, the JBoss Modules documentation still makes a distinction about what a module name is (https://jboss-modules.github.io/jboss-modules/manual/#names) and what are module identifiers https://jboss-modules.github.io/jboss-modules/manual/#slots

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getModuleIdentifier() is already taken, so ....


public ModuleSpec getModuleSpec() {
return moduleSpec;
}
Expand Down
Loading