mockitoguy
commented
6 years ago Can you write an overview of the change. I have a couple of questions :) In general, I'm looking for every opportunity to simplify the code.
-
Can you make sure the feature is documented at the point of merging the PR? I think that we should start adhering to the principle that every PR has documentation. I'm documenting for 1.0 release and it is a pain to do it afterwards :)
-
I think that better home for createDependencyInfoFile task is ComparePublicationsPlugin instead of JavaLibraryPlugin. This way, we can document how it works in ComparePublicationsPlugin javadoc. Long term, I think that our plugins should be documented in the Javadoc rather than in markdown docs. This way, docs live closer to the code and it easier to keep it up to date (and stay motivated to do so).
-
Why do we need to write any code to parse the dependency-info.json file? If we ensure that the dependency-info.json contents are written in a consistent way we can leverage an existing comparison / diff logic. I'd hope that we can reduce the complexity as much as possible and avoid writing a parser / comparator for the file.
-
Do we need to introduce more zip parsing logic? I thought that we already have all the code in place for that. In the PR it looks like new zip logic was introduced. Again, it seems to me that extra complexity can be avoided. We can also use Gradle's zip features to avoid maintaining our own code.
-
How do you avoid version problem for internal (submodule) dependencies? Do you add secret souse to the parser/filter when we compare the json file? Given that you have tested it with release-example this must work with submodule dependencies (cool!!!).
-
dependency-info.json looks untidy with an empty artifact and is inconsistent with Gradle model. AFAIR, Gradle returns 'null' when there is no artifact. Let's change the metadata format to something like this:
{
"artifact": null,
"name":"diffutils",
"version":"1.3.0",
"group":"com.googlecode.java-diff-utils"
} -
To simplify comparison, perhaps we want to avoid JSON and have a flat list / yml. This way, we don't ever parse the file, we diff it textually as every other file in the sources jar. Examples:
- yaml:
description: xxx dependencies: # standard dependencies: - com.googlecode.java-diff-utils:diffutils:1.3.0
- com.gradle.publish:plugin-publish-plugin:0.9.7
internal dependencies:
- com.gradle.publish:plugin-publish-plugin
dependency with artifact
- dependency:
module: foo:bar:1.0
artifacts:
name:classifier:type:extension notation for artifacts
- foo:sources:jar:jar
- foo:sources:jar:jar
- yaml:
just text:
#description xxx
#dependencies
com.googlecode.java-diff-utils:diffutils:1.3.0
com.gradle.publish:plugin-publish-plugin:0.9.7
#internal dependency, no version
foo:bar
#dependency with artifact
foo:bar:1.0
#name:classifier:type:extension notation for artifacts
foo:sources:jar:jarBTW, the code is greatly simplified when we don't have to parse the POM. BIG THANKS for working on this!!!
wwilk
We can now use dependency-info.json for comparing publications. The file started to be generated in Shipkit 0.9.122.
Tested in shipkit and shipkit-example by running "./gradlew assertReleaseNeeded".