gnodet-bot commented on code in PR #13036:
URL: https://github.com/apache/maven/pull/13036#discussion_r4047348988
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Node.java:
##########
@@ -40,39 +40,98 @@
@Provider
public interface Node {
+ /**
+ * Returns the artifact for this node.
+ *
+ * @return artifact for this node, or {@code null} if none
+ */
+ @Nullable
+ Artifact artifact();
+
+ /**
+ * Returns the dependency for this node.
+ *
+ * @return dependency for this node, or {@code null} if none
+ */
+ @Nullable
+ Dependency dependency();
+
+ /**
+ * Gets the child nodes of this node.
+ *
+ * @return the child nodes of this node, never {@code null}
+ */
+ @Nonnull
+ List<Node> children();
+
+ /**
+ * Returns the remote repositories of this node.
+ *
+ * @return repositories of this node, never {@code null}
+ */
+ @Nonnull
+ List<RemoteRepository> remoteRepositories();
+
+ /**
+ * Returns the remote repository from which this artifact was downloaded,
if known.
+ *
+ * @return an {@code Optional} containing the repository, or empty if not
available (e.g. local artifact or root node)
+ */
+ @Nonnull
+ Optional<RemoteRepository> repository();
Review Comment:
⚠️ **Inaccurate Javadoc — NOT addressed from previous review.**
The Javadoc says `@return the repository, never {@code null}`, but:
1. The method returns `Optional<RemoteRepository>` — `never null` is
misleading framing for an Optional (it implies a non-null value, not an
Optional wrapper)
2. `DefaultNode.repository()` still throws
`UnsupportedOperationException("Not implemented yet")` in the current PR HEAD
(9c98b45)
PR #13153 was merged on 2026-09-16 and implemented
`DefaultNode.repository()`, but this PR hasn't been rebased since — the
implementation fix doesn't land here. More importantly, the Javadoc on
`Node.repository()` itself was **downgraded** by this PR: master had `@return
an Optional containing the repository, or empty if not available (e.g. local
artifact or root node)` on `getRepository()`, which this commit replaced with
the less accurate `@return the repository, never null`.
```suggestion
/**
* Returns the remote repository from which this artifact was
downloaded, if known.
*
* @return an {@code Optional} containing the repository, or empty if
not available (e.g. local artifact or root node)
*/
@Nonnull
Optional<RemoteRepository> repository();
```
##########
api/maven-api-core/src/main/java/org/apache/maven/api/LocalRepository.java:
##########
@@ -46,6 +46,23 @@
@Immutable
public interface LocalRepository extends Repository {
+ /**
+ * Returns the path of this local repository.
+ *
+ * @return the path, never {@code null}
+ */
@Nonnull
- Path getPath();
+ Path path();
+
+ /**
+ * Returns the path of this local repository.
+ *
+ * @return the path, never {@code null}
+ * @deprecated Use {@link #path()} instead.
+ */
+ @Nonnull
Review Comment:
🔹 **Nit — inconsistent annotation ordering.**
`@Nonnull` appears before `@Deprecated` here, while every other deprecated
wrapper in this PR has `@Deprecated` first, `@Nonnull` second (e.g.
`Artifact.getGroupId()`, `Artifact.getClassifier()`, etc.).
```suggestion
@Deprecated(since = "4.1.0", forRemoval = true)
@Nonnull
default Path getPath() {
return path();
}
```
--
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]
