Skip to content

Upgrade checkstyle and spotbugs tooling#2699

Open
8Keep wants to merge 1 commit intojMonkeyEngine:masterfrom
8Keep:checkstyle
Open

Upgrade checkstyle and spotbugs tooling#2699
8Keep wants to merge 1 commit intojMonkeyEngine:masterfrom
8Keep:checkstyle

Conversation

@8Keep
Copy link
Copy Markdown
Contributor

@8Keep 8Keep commented Apr 8, 2026

  1. Upgrade spotbugs and checkstyle
  2. Run them both as part of CI, but make them non blocking. Upload the logs so we can take a look whenever.
  3. Move versioning into the unified version file just like everything else.

onlyIf { gradle.rootProject.hasProperty('signingKey') }
}

def checkstyleSupported = JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_21)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New version of checkstyle requires jdk 21+.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the versions for Checkstyle and Spotbugs and adjusts their task configurations, including a transition from 'enabled' to 'required' for Spotbugs reports. It also introduces a Java version check to conditionally enable Checkstyle tasks. Feedback suggests that the Java 21 requirement is too restrictive and potentially incompatible with older Gradle versions, recommending a baseline of Java 17 instead. Furthermore, it is recommended to apply a similar version guard to Spotbugs tasks to prevent build failures on older JDKs.

Comment on lines 52 to 57
tasks.withType(com.github.spotbugs.snom.SpotBugsTask ) {
reports {
html.enabled = !project.hasProperty("xml-reports")
xml.enabled = project.hasProperty("xml-reports")
html.required = !project.hasProperty("xml-reports")
xml.required = project.hasProperty("xml-reports")
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Spotbugs tasks are not guarded by a Java version check, unlike the Checkstyle tasks in common.gradle. Since Spotbugs 4.8+ and its Gradle plugin 6.x require at least Java 17, running these tasks on older JDKs will cause the build to fail. Adding an enabled check ensures the tasks are skipped gracefully on unsupported environments, which aligns with the "non-blocking" goal stated in the PR description.

        tasks.withType(com.github.spotbugs.snom.SpotBugsTask) {
            enabled = JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_17)
            reports {
                html.required = !project.hasProperty("xml-reports")
                xml.required = project.hasProperty("xml-reports")
            }
        }

@8Keep 8Keep marked this pull request as ready for review April 8, 2026 02:38
@8Keep 8Keep added buildscript An issue with the buildscript dependencies Pull requests that update a dependency file labels Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildscript An issue with the buildscript dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant