Conversation
|
@nedtwigg As stated above, currently the problem has no effect on the behaviour. Do you anyhow want an update of the Sorry about the commit message. Will change it when merging. |
fvgh
requested review from
lutovich and
nedtwigg
and removed request for
lutovich
nedtwigg
left a comment
nedtwigg
left a comment
There was a problem hiding this comment.
I have a minor suggestion for making the tests a little easier to read, but the substance of the change looks good to me.
| Set<File> files = provisioner.provisionWithDependencies("foo", "bar", "baz"); | ||
|
|
||
| assertThat(files).containsOnly(new File("foo-1"), new File("foo-2"), new File("baz-2")); | ||
| private void resolveDependencies() throws Exception { |
There was a problem hiding this comment.
I was a little confused about what these tests were doing at first. Maybe rename this to assertFormattingWorks. It would also be a little more robust if it didn't just run, but also asserted that the file on disk was as expected. Maybe assertFormattingWorks(String unformatted, String formatted).
lutovich
left a comment
lutovich
left a comment
There was a problem hiding this comment.
Looks good to me! Made a couple tiny comments.
| return mavenCoords -> mavenCoords.stream() | ||
| .flatMap(coord -> artifactResolver.resolve(coord).stream()) | ||
| .collect(toSet()); | ||
| return mavenCoords -> artifactResolver.resolve(mavenCoords); |
There was a problem hiding this comment.
Maybe artifactResolver::resolve
There was a problem hiding this comment.
@lutovich Sorry, maybe it's just to late... I lost you. Can you explain in detail?
There was a problem hiding this comment.
I just mean that lambda
return mavenCoords -> artifactResolver.resolve(mavenCoords);can probably be replaced with method reference
return artifactResolver::resolve;Then Objects.requireNonNull(artifactResolver); can also be removed because method ref will fail with NPE anyway.
3 participants
Currently Spotless uses 2 kinds of dependencies:
The
Provisionerinterface supports "dependency lists" (currently always resolving transitives).The previous
MavenProvisionerimplementation took individually each dependency as a root dependency, resolves it (and its transitives) and puts the JARs into a hash set.There had been quite an evolution how Maven resolves transitives. Please refer to the Transitive Dependencies documentation for details.
The whole issue is NOT affecting the current formatters (due to the simple dependency structure of current formatters). However, the
maven-pluginProvisionershould not interfere with the dependency resolution ofMavento avoid bugs in future evolution ofSpotless.