gnodet-bot commented on code in PR #13153:
URL: https://github.com/apache/maven/pull/13153#discussion_r4024310888
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Node.java:
##########
@@ -68,6 +68,8 @@ public interface Node {
/**
* The repository where this artifact has been downloaded from.
Review Comment:
💬 **Javadoc summary too narrow.** The summary sentence "The repository where
this artifact has been downloaded from" doesn't reflect the contract for root
nodes or locally-installed artifacts. The new `@return` tag covers it, but the
summary itself will appear in Javadoc tool-tips and IDE hovers stripped of
`@return`. Suggest aligning them:
```suggestion
* Returns the remote repository from which this artifact was
downloaded, if known.
```
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultNode.java:
##########
@@ -72,8 +74,15 @@ public List<RemoteRepository> getRemoteRepositories() {
@Override
public Optional<RemoteRepository> getRepository() {
- // TODO: v4: implement
- throw new UnsupportedOperationException("Not implemented yet");
+ org.eclipse.aether.artifact.Artifact artifact = node.getArtifact();
Review Comment:
🔧 **`node.getRepositories()` called twice.** The list is fetched on line 78
(guard) and again on line 82 (passed to `LocalArtifactRequest`). For
`DefaultDependencyNode` this is trivially cheap, but it's cleaner to capture
once:
```suggestion
org.eclipse.aether.artifact.Artifact artifact = node.getArtifact();
List<org.eclipse.aether.repository.RemoteRepository> repos =
node.getRepositories();
if (artifact == null || repos.isEmpty()) {
```
Then use `repos` in the `LocalArtifactRequest` constructor call. Since
`Node` is `@Immutable` the result is stable, so a local variable is fine.
--
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]
