Launch/Diff Groovy should remove use of maven-checkstyle-plugin
Opened this issue · 10 comments
See https://issues.apache.org/jira/browse/MCHECKSTYLE-346 .
We need to deprecate all usage of maven-checkstyle-plugin so we can start implementing backward breaking changes.
The groovy scripts are our heaviest scripts as we use them for all types of regression, so they should be the first ones converted.
Instead of installing checkstyle and then running mvn site, we should package the project into the all jar and run the CLI checkstyle program and produce the XML violation file. This file and the sources will be what is fed into patch-diff-report-tool to produce the final report.
We should not need to change patch-diff-report-tool. Using the CLI allows us to run Checkstyle on the repo directory directly instead of copying the files to another location temporarily. This will also allow us to run regression on non-Java files which is what maven-jxr-plugin forced on us.
I am ok create issue on Checkstyle to generate HTML report, I think we already have such issue, and raised this in discussions several times.
Or users/me can use xsl to convert xml to html (very old fashioned approach).
Html generation is better to be done html template libraries but it is new dependency .... so it is ok to generate html by old-fashioned way (StringBuilder.append)
maven-jxr-plugin
this is plugin that we benefit a lot, to convert sources to html. It(as a tool) might be used separately to just convert folder of source to folder of html .
I am ok create issue on Checkstyle to generate HTML repor
For regression, we don't need this. patch-diff-report-tool creates the final HTML report for us. We don't use the output of maven-jxr-plugin for anything.
So HTML generation from Checkstyle would have to be a separate need then this.
we don't use the output of maven-jxr-plugin for anything.
this is result of maven-jxr-plugin -
https://djydewang.github.io/diffReport_Issue4981/findbugs-with-excldues/xref/home/bbg/project/contribution/checkstyle-tester/repositories/findbugs-with-excldues/eclipsePlugin/src/edu/umd/cs/findbugs/plugin/eclipse/quickfix/util/SourceLineVisitor.java.html#L26 , without generation of such pages, html report is usless for us , to my mind.
We should be able to run it on sources (with fake pom.xml) to generate folder of html pages.
this is result of maven-jxr-plugin
patch-diff-report-tool generates this via JavaCodeTransform and TextTransform. This is why we have these code transforms in the tool.
https://github.com/checkstyle/contribution/blob/master/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/XrefGenerator.java#L136-L160
If you look at the tool's test resource folder, we only give it Java files as inputs, not html files of the generated source. https://github.com/checkstyle/contribution/tree/master/patch-diff-report-tool/src/test/resources/run . We don't verify the results of the html generation for the Java sources.
If you at look inputs to patch-diff-report-tool at https://github.com/checkstyle/contribution/blob/master/patch-diff-report-tool/README.md refFiles is given the src/main/java folder, not the target folder.
The tool takes the xml checkstyle violation file as input, which has direct links to the source files that caused violations.
I still see usage of plugin in pom.xml - https://github.com/checkstyle/contribution/blob/master/checkstyle-tester/pom.xml#L59
I think site plugin can be removed for sure as we use HTML generation by our diff tool.
@nmancus1 , please make summary of where we are now.
@nmancus1 , please make summary of where we are now.
We need to generate reports from checkstyle directly, but this issue is last on the list of improvements to checkstyle-tester. I have left a general update here.
reminder: checkstyle do not have html reports, chekcstyle can generate xml report and diff.groovy can convert it to HTML.
-Dassembly.skipAssembly=true has to be added to the assembly of the all jar to skip creating the extra files that are not needed.