Skip to content

Added Checkstyle GitHub Actions #2124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rjhancock wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added Checkstyle GitHub Actions #2124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/checkstyle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: Checkstyle

on:
pull_request:
branches: [ main ]

jobs:
pr_build:
runs-on: ${{ matrix.os }}

strategy:
matrix:
os: [ ubuntu-latest ]
java-distribution: [ temurin ]
java-version: [ 25 ]
fail-fast: false

steps:
- name: "Check out MegaMek Lab"
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
uses: actions/checkout@v6

Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
- name: "Check out MegaMek"
uses: actions/checkout@v6
with:
repository: MegaMek/megamek
path: megamek

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
- name: "Check out mm-data"
uses: actions/checkout@v6
with:
repository: MegaMek/mm-data
path: mm-data

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
- name: Setup Java
uses: actions/setup-java@v4
with:
distribution: ${{ matrix.java-distribution }}
java-version: ${{ matrix.java-version }}

Copilot uses AI. Check for mistakes.
- name: Setup Gradle
uses: gradle/actions/setup-gradle@v5

- name: Checkstyle
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
- name: Checkstyle
- name: Checkstyle
working-directory: megameklab

Copilot uses AI. Check for mistakes.
run: ./gradlew checkstyleMain

- name: Upload Test Logs on Failure
uses: actions/upload-artifact@v6
if: failure()
with:
name: cd-failure-logs
path: ./megamek/megamek/build/reports/
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
path: ./megamek/megamek/build/reports/
path: ./megameklab/build/reports/

Copilot uses AI. Check for mistakes.
44 changes: 23 additions & 21 deletions config/checkstyle/checkstyle.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,44 @@

<module name="Checker">
<module name="SuppressionFilter">
<property name="file" value="${suppressionFile}"/>
<property name="file" value="${suppressionFile}" />
</module>

<module name="RegexpSingleline">
<property name="format" value="^(?!.*(bg\.battletech|MechWarrior Copyright|MechWarrior, BattleMech, `Mech and AeroTech are registered trademarks)).*\b(?i)(mech|mechwarrior|battlemech|aerotech|protomech|'mech|`Mech|TripodMech|BipedMech|QuadMech|LandAirMech|OmniMech|’Mechs)\b.*$"/>
<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.*$" />
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
<property name="ignoreCase" value="true" />
<property name="message" value="Forbidden word found" />
<property name="fileExtensions" value="properties, md" />
<property name="severity" value="error" />
</module>

<!-- Check that "license" appears at least once in the file -->
<module name="RegexpSingleline">
<property name="format" value="(?i)\b(license|copyright)\b"/>
<property name="minimum" value="1"/>
<property name="maximum" value="9999"/>
<property name="ignoreCase" value="true"/>
<property name="message" value="File must contain a license header"/>
<property name="severity" value="warning"/>
<property name="fileExtensions" value="java"/>
<property name="format" value="(?i)\b(license|copyright)\b" />
<property name="minimum" value="1" />
<property name="maximum" value="9999" />
<property name="ignoreCase" value="true" />
<property name="message" value="File must contain a license header" />
<property name="severity" value="error" />
<property name="fileExtensions" value="java" />
</module>

<module name="TreeWalker">
<module name="SuppressWithNearbyCommentFilter">
<property name="commentFormat" value="CHECKSTYLE IGNORE ForbiddenWords FOR (\d+) LINES"/>
<property name="checkFormat" value=".*"/>
<property name="influenceFormat" value="$1"/>
<property name="commentFormat" value="CHECKSTYLE IGNORE ForbiddenWords FOR (\d+) LINES" />
<property name="checkFormat" value=".*" />
<property name="influenceFormat" value="$1" />
</module>

<module name="SuppressionCommentFilter"/>
<module name="SuppressionCommentFilter" />

<module name="RegexpSinglelineJava">
<property name="id" value="ForbiddenWords"/>
<property name="format" value="^(?!.*(bg\.battletech|MechWarrior Copyright|MechWarrior, BattleMech, `Mech and AeroTech are registered trademarks)).*\b(?i)(mech|mechwarrior|battlemech|aerotech|protomech|'mech|`Mech|TripodMech|BipedMech|QuadMech|LandAirMech|OmniMech|’Mechs)\b.*$"/>
<property name="ignoreCase" value="true"/>
<property name="ignoreComments" value="false"/>
<property name="message" value="Forbidden word found"/>
<property name="id" value="ForbiddenWords" />
<property name="format" value="^(?!.*(bg\.battletech|MechWarrior Copyright|MechWarrior, BattleMech, `Mech and AeroTech are registered trademarks)).*\b(?i)(mech|mechwarrior|battlemech|aerotech|protomech|'mech|`Mech|TripodMech|BipedMech|QuadMech|LandAirMech|OmniMech|’Mechs)\b.*$" />
<property name="ignoreCase" value="true" />
<property name="ignoreComments" value="false" />
<property name="message" value="Forbidden word found" />
<property name="severity" value="error" />
</module>
</module>
</module>
Loading