-
Notifications
You must be signed in to change notification settings - Fork 21
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
CLM-18313 Builds the artifact ID for InnerSource dependency manually #73
CLM-18313 Builds the artifact ID for InnerSource dependency manually #73
Conversation
ModuleVersionIdentifier artifactId = resolvedArtifact.getModuleVersion().getId(); | ||
|
||
Artifact artifact = new Artifact() | ||
.setId(artifactId.getGroup() + ":" + artifactId.getName() + ":" + artifactId.getVersion()) | ||
.setPathname(resolvedArtifact.getFile()) | ||
.setMonitored(true); | ||
|
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.
Should there be a unit test to test this part? that the artifact has the expected outcome?
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.
Should there be a unit test to test this part? that the artifact has the expected outcome?
There are already tests checking the output is group:name (artifactId):version
For instance:
https://github.com/sonatype-nexus-community/scan-gradle-plugin/blob/master/src/test/java/org/sonatype/gradle/plugins/scan/common/DependenciesFinderTest.java#L177
The case the customer presents couldn't be reproduced (getting extra info in version for SNAPSHOT components) using the current approach, but concatenating those fields manually proved to provide a proper result as per the output they provided in the latest reply by mail.
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.
+1 LGTM
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.
+1
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.
+1, LGTM
ModuleVersionIdentifier artifactId = resolvedArtifact.getModuleVersion().getId(); | ||
|
||
Artifact artifact = new Artifact() | ||
.setId(artifactId.getGroup() + ":" + artifactId.getName() + ":" + artifactId.getVersion()) |
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.
Does the ModuleVersionIdentifier
has any info on classifier
and extension
?
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.
Does the
ModuleVersionIdentifier
has any info onclassifier
andextension
?
Nope!
Only group, name and version: https://docs.gradle.org/current/javadoc/org/gradle/api/artifacts/ModuleVersionIdentifier.html
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.
+1
A Nexus IQ customer is having troubles having InnerSource working properly. After analyzing the files shared by the customer we see the "artifact ID" has a colon (:) in the version fragment for SNAPSHOT dependencies.
By manually building this ID the idea is to avoid any extra characters that a might be introduce in the display name of a component identifier by Gradle.
cc @bhamail / @DarthHater / @guillermo-varela / @shaikhu