jenkinsci / gradle-jpi-plugin

Build Jenkins Plugins with Gradle
79 stars 50 forks source link

Apply Checkstyle, SpotBugs, Jacoco plugins #220

Closed alextu closed 1 year ago

alextu commented 1 year ago

This fixes https://github.com/jenkinsci/gradle-jpi-plugin/issues/217

Apply Checkstyle, SpotBugs, Jacoco plugins with some sensitive defaults:

Note that SpotBugs is only compatible with Gradle 7 and above, running with Gradle 6 will just not apply the SpotBugs plugin.

sghill commented 1 year ago

Hey @alextu wanted to get your thoughts on strategies to avoid Gradle warnings when auto-applying community plugins. I may be a bit of an outlier here, but I like to run with org.gradle.warning.mode=fail to ensure I'm not letting warnings go by unnoticed.

One downside of always applying the spotbugs plugin is we end up getting warnings from Gradle, even when it's disabled. I don't think this applies with jacoco and checkstyle because they're included by default and I've never seen a default plugin warn.

In this case the SpotBugsTask warns because it uses the deprecated ClosureBackedAction:

The org.gradle.util.ClosureBackedAction type has been deprecated. This is scheduled to be removed in Gradle 9.0. Consult the upgrading guide for further information: https://docs.gradle.org/8.0.2/userguide/upgrading_version_7.html#org_gradle_util_reports_deprecations
    at org.gradle.util.ClosureBackedAction.<clinit>(ClosureBackedAction.java:41)
...
    at com.github.spotbugs.snom.SpotBugsTask.reports(SpotBugsTask.groovy:448)
    at com.github.spotbugs.snom.SpotBugsTask$reports.call(Unknown Source)
    at org.jenkinsci.gradle.plugins.jpi.JpiPlugin$_configureSpotbugs_closure38.doCall(JpiPlugin.groovy:660)
alextu commented 1 year ago

Yes that's indeed problematic to apply plugins even if not used. I think we could tackle this in various ways: 1) apply them conditionnaly (see previous comment) => not really fond of using once more afterEvaluate 2) instead of boolean flag, use functions => I implemented this quickly https://github.com/jenkinsci/gradle-jpi-plugin/compare/master...alextu:gradle-jpi-plugin:atual/fix-checks and is my prefered choice 3) react to existing plugins, for example project.pluginManager.withPlugin('checkstyle') { // configure checkstyle => without any additional flags, it could be a surprising behavior or even mislead users regarding default behavior of Checkstyle, SpotBugs, Jacoco

What do you think ?

sghill commented 1 year ago

Let's go with option 2, your preference looks good to me!

alextu commented 1 year ago

Great, here's the PR https://github.com/jenkinsci/gradle-jpi-plugin/pull/222

sghill commented 1 year ago

0.48.0 was released today with this change