[ci][test] Modifications to static analysis contribution via Tomasbjerre #5116
Conversation
* Also using project.parent.basedir and avoiding relative paths in pom files. * Filtering out samples.
This moves the static-analysis checks to a standalone profile. Core contributors may run static analysis with: ``` mvn -Pstatic-analysis install ``` The analysis is separated from default functionality to reduce impact to community contributions. SpotBugs/PMD may add a non-trivial amount of time to builds on some machines.
|
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
|
|
||
| <!-- For source/class naming, see https://spotbugs.readthedocs.io/en/stable/filter.html#java-element-name-matching --> | ||
| <Match> | ||
| <!-- Ignore all source files with samples in path name --> |
There was a problem hiding this comment.
One could argue that the generated code should also be free from issues found by static code analysis. I'm not sure about excluding samples here.
I can see how users have different code standards and running Checkstyle and PMD is not worth the effort, but I'm not sure it is good to exclude Spotbugs here.
There was a problem hiding this comment.
Another reason to exclude these here is that I assume most users use custom templates. Analyzing our "samples" is therefore very much like analyzing our tests.
There was a problem hiding this comment.
Another concern is that only a very small subset of our samples are Java :
Maybe we could later work out the pattern to exclude everything but java client and server outputs in the samples directory.
| <configuration> | ||
| <failOnError>false</failOnError> | ||
| <!-- https://spotbugs.readthedocs.io/en/stable/effort.html --> | ||
| <effort>min</effort> |
There was a problem hiding this comment.
Now that it does not run by default, perhaps it does not need to be min?
There was a problem hiding this comment.
I can evaluate this once it's hooked into the target pipeline.
|
The But perhaps you would rather have Sonar do the commenting. |
We have some cleanup to do with respect to how much is being logged in Travis. As it is, both Travis and CirciCI UIs lock up my browser because of the amount of text output. I'm looking at modifying our logger to reduce duplicated log lines, and once that's done I will look out the outputs in other CI. I'm thinking the |
This extends on submission by @tomasbjerre in #5040 by moving the PMD and Spotbugs plugin execution to a separate profile
static-analysis. This keeps CheckStyle in the default build because it added a very minimal amount of time to execution.Once merged, I'd like to extend the Sonar CI GitHub Action to be a static analysis action which caches the xml reports generated by these plugins as build artifacts. This will give us SonarCloud as a means for evaluating the static analysis, and allow for contributors to download the
*.xmlreports from the action. Once the GitHub action is updated, we can also close #33 as well.Core contributors may run static analysis with:
The analysis is separated from default functionality to reduce impact to community contributions. SpotBugs/PMD may add a non-trivial amount of time to builds on some machines.
cc @OpenAPITools/generator-core-team
Closes #5040
PR checklist
./bin/(or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run./bin/{LANG}-petstore.sh,./bin/openapi3/{LANG}-petstore.shif updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).master,4.3.x,5.0.x. Default:master.