gnodet-bot commented on code in PR #13501:
URL: https://github.com/apache/maven/pull/13501#discussion_r4269642223
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Exclusion.java:
##########
@@ -41,10 +46,15 @@ default String getGroupId() {
return groupId();
}
+ /**
+ * {@return the artifact identifier of the excluded dependency, or {@code
null} for a wildcard}.
+ */
@Nullable
String artifactId();
/**
+ * {@return the artifact identifier of the excluded dependency, or {@code
null} for a wildcard}.
Review Comment:
💡 Same duplication for `getArtifactId()` — the `{@return}` text is already
on `artifactId()`. Consider keeping only the `@deprecated` tag here as well:
```suggestion
* @deprecated Use {@link #artifactId()} instead.
```
##########
api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlNode.java:
##########
@@ -343,6 +343,17 @@ static XmlNode merge(@Nullable XmlNode dominant, @Nullable
XmlNode recessive) {
return XmlService.merge(dominant, recessive, null);
}
+ /**
+ * Merge recessive into dominant, with an explicit child merge override.
+ *
+ * @param dominant the dominant node, may be {@code null}
+ * @param recessive the recessive node, may be {@code null}
+ * @param childMergeOverride if non-null, overrides the child combination
mode
+ * @return the merged node, or {@code null} if both inputs are {@code null}
+ *
+ * @deprecated Use {@link XmlService#merge(XmlNode, XmlNode, Boolean)}
instead.
+ */
+ @Deprecated(since = "4.0.0", forRemoval = true)
Review Comment:
⚠️ This adds a new `@Deprecated(since="4.0.0", forRemoval=true)` annotation
to a method that had none before — this is a behavioural/API change, not just
documentation. Any caller compiled with `-Werror` (or `javac
-Xlint:deprecation` + `-Werror`) will break. Is the intent to formally start
the removal clock on this 3-arg `merge()` overload? If so, it should be called
out explicitly in the PR description and changelog.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Exclusion.java:
##########
@@ -29,10 +29,15 @@
*/
@Experimental
public interface Exclusion {
+ /**
+ * {@return the group identifier of the excluded dependency, or {@code
null} for a wildcard}.
+ */
@Nullable
String groupId();
/**
+ * {@return the group identifier of the excluded dependency, or {@code
null} for a wildcard}.
Review Comment:
💡 The `{@return}` sentence here duplicates the Javadoc already on
`groupId()` verbatim. For deprecated bridge methods the convention is to keep
only the `@deprecated` tag (and optionally a `@see` or `{@link}` to the
replacement) — not copy the full description:
```suggestion
* @deprecated Use {@link #groupId()} instead.
```
--
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]
