-
Notifications
You must be signed in to change notification settings - Fork 161
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.
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
[ARCHETYPE-655] Get rid of Wagon API to download catalogs #215
Conversation
f1801dd
to
a77eeb4
Compare
a77eeb4
to
12fd3ab
Compare
@cstamas can you look? |
|
||
if (session != null && id != null) { | ||
MavenExecutionRequest request = session.getRequest(); | ||
MetadataRequest request = new MetadataRequest(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat trick, but I'd rather do it like here:
https://github.com/apache/maven/blob/master/maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultTransportProvider.java
and
https://github.com/apache/maven/blob/master/maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultTransportProvider.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I'd not use metadata op to resolve catalog, as catalog is really not part of Resolver "layout" (and Resolver is oblivious about it). Here we deal with a resource "that happens" that is somewhere in remote repo (root), so I'd use same approach as in Maven4 Transporter implementation: direct use of Transport resolver interface.
Don't get me wrong, I am not -1 on this PR, but anyone looking at the code will raise eyebrows when look into how is remote catalog fetched 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, I am now contradicting myself: this PR is ok, and I myself was not aware of such use of Metadata, kudos. +1.
Leave it as is pls!
https://issues.apache.org/jira/browse/ARCHETYPE-655