Conversation
Godin
left a comment
There was a problem hiding this comment.
In addition to our CI, I also did some tests locally and do not observe any differences.
So I really like this change - same functionality, almost the same code, less dependencies 👍
Also I tried to upgrade ECJ and use it for compilation into bytecode version higher than 8, what fails without this change
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.7.0:compile (default-compile) on project jacoco-maven-plugin: Compilation failure: Compilation failure:
[ERROR] /Users/godin/projects/jacoco/jacoco/jacoco-maven-plugin/target/generated-sources/plugin/HelpMojo.java:[8,215] The package org.w3c.dom is accessible from more than one module: <unnamed>, java.xml
while after this PR succeeds 👍
@marchof are you ok if I merge it without change of org.jacoco.doc/docroot/doc/changes.html ?
| <li>Maven plug-in has no dependency on <code>maven-reporting-impl</code> any more | ||
| (GitHub <a href="https://github.com/jacoco/jacoco/issues/1121">#1121</a>).</li> |
There was a problem hiding this comment.
To me this is pure internal non-functional change, so I do not think that we need this changelog entry and Git commit message is enough. Similar #645 wasn't mentioned either.
There was a problem hiding this comment.
I added it as it solves two tickets. But I also agree with your point of view.
This dependency is not really useful for JaCoCo reports and has several
transitive dependencies where security vulnerabilities have been
reported.
Fixes #641, #920