[GitHub] [maven] michael-o commented on a change in pull request #643: [MNG-5561] Plugin relocation loses configuration
michael-o commented on a change in pull request #643: URL: https://github.com/apache/maven/pull/643#discussion_r776673637 ## File path: maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/RelocatedArtifact.java ## @@ -86,6 +86,39 @@ public String getVersion() } } +@Override Review comment: Yes, we are using the new object. Checked myself and tripped over. I can drop the `@Override`, no issue. Another issue was also that the outcome was *not* a `RelocatedArtifact`, but a `DefaultArtifact`. This means that the original information was lost. -- 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: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #643: [MNG-5561] Plugin relocation loses configuration
michael-o commented on a change in pull request #643: URL: https://github.com/apache/maven/pull/643#discussion_r776673637 ## File path: maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/RelocatedArtifact.java ## @@ -86,6 +86,39 @@ public String getVersion() } } +@Override Review comment: Yes, we are using the new object. Checked myself and tripped over. I can drop the `@Override`, no issue. -- 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: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #643: [MNG-5561] Plugin relocation loses configuration
michael-o commented on a change in pull request #643: URL: https://github.com/apache/maven/pull/643#discussion_r776670895 ## File path: maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/RelocatedArtifact.java ## @@ -86,6 +86,39 @@ public String getVersion() } } +@Override +public Artifact setVersion( String version ) +{ + String current = getVersion(); + if ( current.equals( version ) || ( version == null && current.length() <= 0 ) ) Review comment: Negative (black) magic. -- 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: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #643: [MNG-5561] Plugin relocation loses configuration
michael-o commented on a change in pull request #643: URL: https://github.com/apache/maven/pull/643#discussion_r776669482 ## File path: maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/RelocatedArtifact.java ## @@ -86,6 +86,39 @@ public String getVersion() } } +@Override +public Artifact setVersion( String version ) +{ + String current = getVersion(); + if ( current.equals( version ) || ( version == null && current.length() <= 0 ) ) Review comment: It can't. You see this pattern everywhere in Maven space. I simply copied the code from the parent class. I intentionally did not want to change it in this spot. It should be changed all at once everywhere. -- 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: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org