Add a PinnedMavenInstallAnalyzer#4773
Conversation
boring-cyborg
Bot
added
ant
dhalperi
left a comment
dhalperi
left a comment
There was a problem hiding this comment.
FYI, I modeled this after PipAnalyzer, with a lot of copy and paste.
I did follow my own standards for implementation details and debug logging. E.g., I deleted overridden Javadoc identical to parent.
I anticipate and am happy to iterate on the code to bring it up to local standards.
dhalperi
force-pushed
the
maven-install
branch
from
80a20fc to
bbe3147
Compare
dhalperi
force-pushed
the
maven-install
branch
3 times, most recently
from
6c2d5de to
9a00290
Compare
`install.json` is a new type of Maven lockfile commonly used in Bazel Java projects. Implement virtual dependency scanning for such files, modeled after the existing PipAnalyzer. In addition to the testing added in this PR, it worked on our install.json file: https://github.com/batfish/batfish/blob/6688b5b49ea695e7b566a0b70403396f580b2805/maven_install.json
dhalperi
force-pushed
the
maven-install
branch
from
9a00290 to
0c28bd6
Compare
I'll leave the codestyle review to the various automated code reviews... results should appear soon now that I triggered the PR workflow. Hope to take a second look at the coding over the weekend. @jeremylong any not automatically checked coding standards you'd like to enforce on the project? |
|
@dhalperi Up to now no issue, I've only glanced over and picked out some items, but going forward: please do not force-push, as that breaks the review cycle. With regular pushes only the changed bits need (re)review, with a force-push the entire change needs to be (re)reviewed as the delta to the already reviewed code is unclear. |
…venInstallAnalyzer.java Co-authored-by: Jeremy Long <[email protected]>
|
Thanks for all the great feedback and useful improvements! What else can I do? |
|
@dhalperi thanks for the PR! |
3 participants
Fixes Issue
#4772
Description of Change
install.jsonis a new type of Maven lockfile commonly usedin Bazel Java projects. Implement virtual dependency scanning
for such files, modeled after the existing PipAnalyzer.
Have test cases been added to cover the new functionality?
Yes, I think so.
However, I was not able to run
mvn -s settings.xml verifyto success locally, so I did not yet add an IT. I also anticipate some changes to these tests in review.In addition to the testing added in this PR, it worked on our
install.json file:
https://github.com/batfish/batfish/blob/6688b5b49ea695e7b566a0b70403396f580b2805/maven_install.json