Added Checkstyle GitHub Actions #2124
Conversation
Added Checkstyle to MM Data
There was a problem hiding this comment.
Pull request overview
This PR adds a GitHub Actions workflow for Checkstyle validation and updates the Checkstyle configuration. The workflow runs on pull requests to the main branch to enforce license compliance and forbidden word checks.
Changes:
- Added new GitHub Actions workflow (
.github/workflows/checkstyle.yml) to run Checkstyle on pull requests - Updated Checkstyle configuration with formatting consistency (added spaces before closing XML tags)
- Added "BattleMech Manual" to the forbidden words exclusion list for properties and markdown files
- Changed license check severity from "warning" to "error" for stricter enforcement
- Added explicit "error" severity to forbidden words checks
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
.github/workflows/checkstyle.yml |
New workflow that runs Checkstyle validation on pull requests to main branch using Java 25 and Gradle |
config/checkstyle/checkstyle.xml |
Formatting updates (spacing), added "BattleMech Manual" exclusion, and changed severity levels from warning to error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| steps: | ||
| - name: "Check out MegaMek Lab" | ||
| uses: actions/checkout@v6 | ||
|
|
There was a problem hiding this comment.
The workflow is missing the MegaMek dependency checkout. The settings.gradle file (line 2) shows that megamek is an included build (includeBuild('../megamek')), and all other workflows in the repository (ci.yml, code-coverage.yml, codeql-analysis.yml, dependency-scan.yml) checkout the megamek repository before running Gradle tasks. Without this checkout, the Gradle build will fail when trying to resolve the included build dependency.
| - name: "Check out MegaMek" | |
| uses: actions/checkout@v6 | |
| with: | |
| repository: MegaMek/megamek | |
| path: megamek |
| <property name="ignoreCase" value="true"/> | ||
| <property name="message" value="Forbidden word found"/> | ||
| <property name="fileExtensions" value="properties, md"/> | ||
| <property name="format" value="^(?!.*(bg\.battletech|MechWarrior Copyright|MechWarrior, BattleMech, `Mech and AeroTech are registered trademarks|BattleMech Manual)).*\b(?i)(mech|mechwarrior|battlemech|aerotech|protomech|'mech|`Mech|TripodMech|BipedMech|QuadMech|LandAirMech|OmniMech|'Mechs)\b.*$" /> |
There was a problem hiding this comment.
The regex pattern includes "BattleMech Manual" in the exclusion list, which differs from the RegexpSinglelineJava check on line 41 that does not include this exclusion. This creates an inconsistency where the same phrase could be allowed in properties/md files but flagged in Java files. Consider whether both patterns should have the same exclusions for consistency.
| fail-fast: false | ||
|
|
||
| steps: | ||
| - name: "Check out MegaMek Lab" |
There was a problem hiding this comment.
The step name uses "MegaMek Lab" (two words), but the convention in other workflows is "MegaMekLab" (one word) as seen in codeql-analysis.yml:19, dependency-scan.yml:21, and java-doc.yml:26. Consider using the consistent naming convention.
| steps: | ||
| - name: "Check out MegaMek Lab" | ||
| uses: actions/checkout@v6 | ||
|
|
There was a problem hiding this comment.
The workflow is missing the mm-data repository checkout. The settings.gradle file includes mm-data as an included build (line 3), and all other workflows that run Gradle tasks (ci.yml, code-coverage.yml, dependency-scan.yml, java-doc.yml) checkout the mm-data repository. While checkstyleMain may not directly depend on mm-data, omitting this checkout could cause issues with Gradle's composite build configuration.
| - name: "Check out mm-data" | |
| uses: actions/checkout@v6 | |
| with: | |
| repository: MegaMek/mm-data | |
| path: mm-data |
| steps: | ||
| - name: "Check out MegaMek Lab" | ||
| uses: actions/checkout@v6 | ||
|
|
There was a problem hiding this comment.
The workflow is missing a Java setup step. All other workflows in the repository use actions/setup-java to configure the JDK before running Gradle tasks. The checkstyle plugin in build.gradle (line 111) is configured to use toolVersion 12.3.1 which requires Java to be available. Without this step, the Gradle wrapper might not have access to the required Java version.
| - name: Setup Java | |
| uses: actions/setup-java@v4 | |
| with: | |
| distribution: ${{ matrix.java-distribution }} | |
| java-version: ${{ matrix.java-version }} |
| if: failure() | ||
| with: | ||
| name: cd-failure-logs | ||
| path: ./megamek/megamek/build/reports/ |
There was a problem hiding this comment.
The artifact upload path is incorrect. The path points to ./megamek/megamek/build/reports/ but should be ./megameklab/megameklab/build/reports/ to match the actual project structure. When checking out the repository, the code is placed in the root directory (unlike other workflows that checkout to a megameklab subdirectory), and the build reports will be generated at ./megameklab/build/reports/ as seen in other workflows like ci.yml (line 99) and code-coverage.yml (line 55).
| path: ./megamek/megamek/build/reports/ | |
| path: ./megameklab/build/reports/ |
| - name: Setup Gradle | ||
| uses: gradle/actions/setup-gradle@v5 | ||
|
|
||
| - name: Checkstyle |
There was a problem hiding this comment.
The Gradle command is missing a working directory specification. The settings.gradle file shows that the actual megameklab project is in the megameklab/ subdirectory. The command should either include working-directory: megameklab or use ./megameklab/gradlew checkstyleMain to match the project structure seen in other workflows like ci.yml (line 92) and code-coverage.yml (line 48).
| - name: Checkstyle | |
| - name: Checkstyle | |
| working-directory: megameklab |
1 participant
This adds Checkstyle to GitHub Actions to ensure license compliance and forbidden word checks.