gnodet commented on code in PR #1815:
URL: https://github.com/apache/maven-resolver/pull/1815#discussion_r3002635743
##########
maven-resolver-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java:
##########
@@ -201,34 +223,67 @@ public void setVersion(Version version) {
this.version = version;
}
+ @Override
public void setScope(String scope) {
if (dependency == null) {
throw new IllegalStateException("node does not have a dependency");
}
dependency = dependency.setScope(scope);
}
+ @Override
public void setOptional(Boolean optional) {
if (dependency == null) {
throw new IllegalStateException("node does not have a dependency");
}
dependency = dependency.setOptional(optional);
}
+ @Override
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Thanks, the backwards-compatible mapping looks good. Much better than
throwing.
##########
maven-resolver-api/src/main/java/org/eclipse/aether/collection/DependencyManagement.java:
##########
@@ -29,22 +30,46 @@
* @see DependencyManager#manageDependency(org.eclipse.aether.graph.Dependency)
*/
public final class DependencyManagement {
+ /**
+ * Enumeration of manageable attributes, attributes that can be subjected
to dependency management.
+ *
+ * @since 2.0.17
+ */
+ public enum Subject {
+ VERSION,
+ SCOPE,
+ OPTIONAL,
+ EXCLUSIONS,
+ PROPERTIES
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Good point about the Java 8 baseline. `EnumMap` is the right choice here.
The remaining `Map<Subject, Object>` type erasure concern is minor given that
all setters are type-safe and getters are the only cast points. Acceptable
trade-off.
##########
maven-resolver-api/src/main/java/org/eclipse/aether/collection/DependencyManagement.java:
##########
@@ -29,22 +30,46 @@
* @see DependencyManager#manageDependency(org.eclipse.aether.graph.Dependency)
*/
public final class DependencyManagement {
+ /**
+ * Enumeration of manageable attributes, attributes that can be subjected
to dependency management.
+ *
+ * @since 2.0.17
+ */
+ public enum Subject {
+ VERSION,
+ SCOPE,
+ OPTIONAL,
+ EXCLUSIONS,
+ PROPERTIES
+ }
- private String version;
-
- private String scope;
-
- private Boolean optional;
-
- private Collection<Exclusion> exclusions;
-
- private Map<String, String> properties;
+ private final Map<Subject, Object> managedValues;
+ private final Map<Subject, Boolean> managedEnforced;
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
`EnumMap` is perfect here — fixed-size internal array, much lighter than
`HashMap`. Thanks.
##########
maven-resolver-api/src/main/java/org/eclipse/aether/graph/DependencyNode.java:
##########
@@ -38,40 +39,49 @@
* @noextend This interface is not intended to be extended by clients.
*/
public interface DependencyNode {
-
/**
* A bit flag indicating the dependency version was subject to dependency
management
*
* @see #getManagedBits()
+ * @deprecated Use {@link #isManagedSubject(DependencyManagement.Subject)}
and {@link #isManagedSubjectEnforced(DependencyManagement.Subject)} instead.
*/
+ @Deprecated
int MANAGED_VERSION = 0x01;
/**
* A bit flag indicating the dependency scope was subject to dependency
management
*
* @see #getManagedBits()
+ * @deprecated Use {@link #isManagedSubject(DependencyManagement.Subject)}
and {@link #isManagedSubjectEnforced(DependencyManagement.Subject)} instead.
*/
+ @Deprecated
int MANAGED_SCOPE = 0x02;
/**
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
The default implementations look correct. The `default` case in the
`isManagedSubject` switch throwing `IllegalArgumentException` is good defensive
coding for potential future enum additions. LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
