[INFRA-2294][INFRA-2000] Enable Warnings Next Generation Plugin #121
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removed FindBugs and CheckStyle maven configuration. Removed old FindBugs and CheckStyle parameters for old plugins. Enabled warnings-ng reporters for Maven, Java and JavaDoc. Enabled warnings-ng reporters for CheckStyle, PMD, and FindBugs. Enabled warnings-ng reporters for Open Tasks.
|
Build failed; the context from the latest run is: Expand to view |
|
Build failed; the context from the latest run is: Expand to view |
spotbugs is run as part of However it will fail the build and we expect an unstable build for testish (of which spotbugs is one) failures, so I think you still need to tell the build not to fail for spotbugs issues. checkstyle is not as actually getting a consensus on any style is problematic :)
There are at least some plugins that do. Actually this is an issue I had recently with wanrings ng - the quality gate is acrross all tools not just specific ones that it used to be (which allows marking unstable for any spotbugs issue, but just reporting TODOs checkstyle etc). https://github.com/search?p=2&q=org%3Ajenkinsci+findbugs+filename%3AJenkinsfile&type=Code shows quite a few (on page 2 onwards) |
|
@jtnord you can set a quality gates for a subset of tools using two |
|
Yes, indeed, you can specify a quality gate for each step, this was one of the main requirements for the warnings-ng redesign. |
|
Does that mean it is ok to remove the |
|
What still sounds for me as pipeline newbie kind of strange is that we need to remodel the parameters of my steps as parameters for the So in pseudo-code it would be nice to have: buildPlugin(
post: () -> {
recordIssues tools: [java(), javaDoc()], aggregatingResults: 'true', id: 'java', name: 'Java'
publishCoverage adapters: [jacocoAdapter('target/site/jacoco/jacoco.xml')]
...
}
) |
but if you want an aggregated report... maybe we where doing something else wrong. I will try and reproduce and ask on the users list
you can do that but it's the old way (not sure of all the reasons) but it can mean a plugin upgrade change could require reconfig of many jobs rather than a central place. |
yes. the failOnError should be the spotbugs flag not findings as that is about to be removed (or use both) |
|
Build failed; the context from the latest run is: Expand to view |
|
Build failed; the context from the latest run is: Expand to view |
|
Build failed; the context from the latest run is: Expand to view |
|
The buildPluginWithGradle step should also be adapted. |
|
Build failed; the context from the latest run is: Expand to view |
|
Build failed; the context from the latest run is: Expand to view |
|
Build failed; the context from the latest run is: Expand to view |
# Conflicts: # vars/buildPlugin.groovy
|
Does someone know how to fix the unit test failure Do I need to register my steps somewhere? |
|
The step works perfectly in our CI build |
|
@uhafner regarding Cheers |
v1v
reviewed
Feb 24, 2020
v1v
previously approved these changes
Feb 26, 2020
|
|
This was referenced Jul 25, 2020
timja
added a commit
to timja/dimensionsscm-plugin
that referenced
this pull request
Jul 25, 2020
Spotbugs reporting is being enabled by default in jenkins-infra/pipeline-library#121, update to parent pom 4.x to use spotbugs instead of findbugs.
This was referenced Jul 25, 2020
Merged
|
@oleg-nenashev @slide @uhafner I've opened PRs to every single plugin that uses the findbugs or checkstyle configurations. are we good to go ahead? |
timja
approved these changes
Jul 25, 2020
|
@timja goes above and beyond again! I'm happy to move forward |
tomasbjerre
pushed a commit
to jenkinsci/violation-comments-to-stash-plugin
that referenced
this pull request
Jul 26, 2020
Spotbugs reporting is being enabled by default in jenkins-infra/pipeline-library#121, update to parent pom 4.x to use spotbugs instead of findbugs.
oleg-nenashev
approved these changes
Jul 27, 2020
chriskilding
pushed a commit
to jenkinsci/aws-secrets-manager-credentials-provider-plugin
that referenced
this pull request
Jul 27, 2020
Checkstyle and Spotbugs were added to the standard build configuration in jenkins-infra/pipeline-library#121.
jglick
reviewed
Aug 5, 2020
| trendChartType: 'TOOLS_ONLY', | ||
| referenceJobName: referenceJobName | ||
| recordIssues enabledForFailure: true, | ||
| tools: [java(), javaDoc()], |
There was a problem hiding this comment.
With the addition of Checks, we are suddenly getting tons and tons of warnings. For example, https://github.com/jenkinsci/ldap-plugin/pull/48/files is practically illegible because of irrelevant, low-priority warnings like
NORMAL:
no @param for rootDN
Worse, checks-api apparently submits annotations even for lines of code which are not modified by a PR and GH displays every annotation, so the Files tab is lost in annotations which are not even related to the diff.
Can we at least turn off display of low-priority Javadoc warnings?
There was a problem hiding this comment.
Hmm, do not seem to be getting these Checks results in PRs on other plugins. Not sure what would be different here.
There was a problem hiding this comment.
You could work around this by doing one of:
- fixing the javadoc issues
- adjusting the maven javadoc plugin not to complain about these
- extending the javadoc / java parts of buildPlugin to allow overriding the configuration like you can do for spotbugs, checkstyle, pmd already and then do something like
buildPlugin(javadoc: skipPublishingChecks: true) in your plugin - turning checks off by default for javadoc
thoughts?
cc @XiongKezhi
There was a problem hiding this comment.
adjusting the maven javadoc plugin not to complain about these
Could be done in plugin-pom I guess. I care about genuine problems—@link target missing, mistyped tag, etc.—but not just silliness like missing @param which is true for most methods in the Jenkins source base.
There was a problem hiding this comment.
Those warnings are also reported by the warnings-ng plugin: https://ci.jenkins.io/job/Plugins/job/ldap-plugin/job/PR-48/3/, they are just scanned from Java or JavaDoc log files, we can't really do much about the scan and collect process. But you can control the minimal severity of issues to report:
recordIssues tool: java(pattern: '*.log'), minimumSeverity: 'HIGH'
But this is the pipeline grammar of warnings-ng, I'm not sure how to override the configurations in buildPlugin, maybe buildPlugin(java: minimumSeverity: 'LOW')? @timja
Also, do you think we should provide another configuration for minimal severity beside the warnings-ng one to only control the published checks?
There was a problem hiding this comment.
I don’t think so, just need to configure warnings ng / the Pom appropriately
chriskilding
pushed a commit
to jenkinsci/aws-secrets-manager-credentials-provider-plugin
that referenced
this pull request
Sep 16, 2020
Checkstyle and Spotbugs were added to the standard build configuration in jenkins-infra/pipeline-library#121.
jetersen
pushed a commit
to jenkinsci/accurev-plugin
that referenced
this pull request
Oct 7, 2020
Spotbugs reporting is being enabled by default in jenkins-infra/pipeline-library#121, update to parent pom 4.x to use spotbugs instead of findbugs.
aakoch
pushed a commit
to aakoch/violation-comments-to-stash-plugin
that referenced
this pull request
Dec 12, 2020
Spotbugs reporting is being enabled by default in jenkins-infra/pipeline-library#121, update to parent pom 4.x to use spotbugs instead of findbugs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will remove the old static analysis configuration and replace it with a configuration of the
Warnings Next Generation plugin. See example output to get an impression of the new configuration.
I removed the old configuration of Checkstyle and Findbugs and replaced it with the corresponding Warnings plugin configuration. There are two open things that need to be discussed (and may be added in another PR):
buildPluginstep. In my jobs I configure all analysis tools in the pom so thatmaven verifywill invoke automatically the static analysis tools that I want (with my configuration files and options). The warnings plugin is smart enough to not break the build if there are no reports written.Detailed changes summary: