Linkage Checker to handle classifier in artifacts#2176
Conversation
| File file = artifact.getFile(); | ||
| if (file != null && file.getName().endsWith(".jar")) { | ||
| String versionlessCoordinates = Artifacts.makeKey(artifact); | ||
| String versionlessCoordinates = Artifacts.makeKey(artifact) + artifact.getClassifier(); |
There was a problem hiding this comment.
Does getClassifier() return the colon prefix, too? Like ":test"?
There was a problem hiding this comment.
No, it doesn't give a colon prefix. Let me add one. I think it's more clear.
dzou
left a comment
There was a problem hiding this comment.
LGTM on my end, will let Elena a chance to review
| LinkageChecker linkageChecker = LinkageChecker.create(bom); | ||
| ImmutableSet<LinkageProblem> linkageProblems = linkageChecker.findLinkageProblems(); | ||
|
|
||
| // bom-with-classifier-artifacts contains com.google.cloud:goole-cloud-core:jar:tests:1.96.0. |
There was a problem hiding this comment.
| // bom-with-classifier-artifacts contains com.google.cloud:goole-cloud-core:jar:tests:1.96.0. | |
| // bom-with-classifier-artifacts contains com.google.cloud:google-cloud-core:jar:tests:1.96.0. |
| graph.addPath(path); | ||
| } | ||
|
|
||
| Verify.verify( |
There was a problem hiding this comment.
Is path != null equivalent to !node.getModuleArtifacts().isEmpty()? If so you may be able to just verify that condition instead which would allow you to avoid defining path outside the loop.
There was a problem hiding this comment.
Updated this to check more earlier.
| LinkageChecker linkageChecker = LinkageChecker.create(bom); | ||
| ImmutableSet<LinkageProblem> linkageProblems = linkageChecker.findLinkageProblems(); | ||
|
|
||
| // bom-with-classifier-artifacts contains com.google.cloud:goole-cloud-core:jar:tests:1.96.0. |
| graph.addPath(path); | ||
| } | ||
|
|
||
| Verify.verify( |
There was a problem hiding this comment.
Updated this to check more earlier.
| return new Dependency(artifact, "compile"); | ||
| } | ||
|
|
||
| private static DependencyGraph createDependencyGraph(ResolvedConfiguration configuration) { |
There was a problem hiding this comment.
Removing static because it now accesses getLogger.
| return graph; | ||
| } | ||
|
|
||
| private static ClassPathResult createClassPathResult(ResolvedConfiguration configuration) { |
There was a problem hiding this comment.
Removing static because it accesses createDependencyGraph, which is now non-static.
| parentPath == null | ||
| ? new DependencyPath(artifactFrom(node)) | ||
| : parentPath.append(dependencyFrom(node)); | ||
| DependencyPath path = null; |
There was a problem hiding this comment.
Just reminder to define this in the loop if it is no longer needed outside.
There was a problem hiding this comment.
This variable is needed outside.
Fixes #2173