Skip to content
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.

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

add workflow, checkstyle and configurations #113

Merged
merged 8 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
54 changes: 54 additions & 0 deletions .github/workflows/checkstyle.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
name: Checkstyle
on:

pull_request:
branches: ["master", "develop"]
paths:
- '**/*.java' # Monitor all Java files

jobs:
checkstyle:
runs-on: ubuntu-latest
permissions: # Add permissions for check enforcement
contents: read
pull-requests: write

steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0 # Required to get changed files

- name: Set up JDK 17
uses: actions/setup-java@v4
with:
java-version: '17'
distribution: 'adopt'

- name: Get Changed Files
id: changed-files
uses: tj-actions/changed-files@v39
with:
files: '**/*.java'

- name: Cache Maven packages
uses: actions/cache@v4
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('/pom.xml') }}
restore-keys: ${{ runner.os }}-maven
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix Maven cache key path.

The cache key is using an incorrect path to pom.xml. The leading slash makes it an absolute path.

-          key: ${{ runner.os }}-maven-${{ hashFiles('/pom.xml') }}
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('/pom.xml') }}
restore-keys: ${{ runner.os }}-maven
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: ${{ runner.os }}-maven


- name: Install Dependencies
run: mvn install -DskipTests

- name: Run Checkstyle on Changed Files
if: steps.changed-files.outputs.any_changed == 'true'
run: |
mvn checkstyle:check -Dcheckstyle.includes="${{ steps.changed-files.outputs}}"

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix Checkstyle execution for changed files.

The current implementation has two issues:

  1. Incorrect usage of changed-files outputs
  2. Template expression evaluation issue flagged by actionlint
-        run: |
-         mvn checkstyle:check -Dcheckstyle.includes="${{ steps.changed-files.outputs}}"
+        run: |
+          changed_files="${{ steps.changed-files.outputs.all_changed_files }}"
+          if [ ! -z "$changed_files" ]; then
+            mvn checkstyle:check -Dcheckstyle.includes="$changed_files"
+          fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Run Checkstyle on Changed Files
if: steps.changed-files.outputs.any_changed == 'true'
run: |
mvn checkstyle:check -Dcheckstyle.includes="${{ steps.changed-files.outputs}}"
- name: Run Checkstyle on Changed Files
if: steps.changed-files.outputs.any_changed == 'true'
run: |
changed_files="${{ steps.changed-files.outputs.all_changed_files }}"
if [ ! -z "$changed_files" ]; then
mvn checkstyle:check -Dcheckstyle.includes="$changed_files"
fi
🧰 Tools
🪛 actionlint (1.7.4)

46-46: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type {string => string}

(expression)

- name: Block PR on Checkstyle Failure
if: failure()
uses: actions/github-script@v7
with:
script: |
core.setFailed('Checkstyle checks failed. Please fix the code formatting issues.')
67 changes: 67 additions & 0 deletions checkstyle.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
"https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
<property name="severity" value="warning"/>
<property name="fileExtensions" value="java"/>

<!-- Checks for Size Violations -->
<module name="FileLength">
<property name="max" value="500"/>
</module>
<module name="LineLength">
<property name="max" value="120"/>
</module>

<module name="TreeWalker">
<!-- Checks for Naming Conventions -->
<module name="MethodName"/>
<module name="PackageName"/>
<module name="TypeName"/>
<module name="ParameterName"/>
<module name="LocalVariableName"/>
<module name="MemberName"/>

<!-- Checks for imports -->
<module name="AvoidStarImport"/>
<module name="IllegalImport"/>
<module name="RedundantImport"/>
<module name="UnusedImports"/>

<!-- Checks for Size Violations -->
<module name="MethodLength">
<property name="max" value="50"/>
</module>

<!-- Checks for whitespace -->
<module name="EmptyLineSeparator"/>
<module name="GenericWhitespace"/>
<module name="MethodParamPad"/>
<module name="NoWhitespaceAfter"/>
<module name="NoWhitespaceBefore"/>
<module name="ParenPad"/>
<module name="TypecastParenPad"/>
<module name="WhitespaceAfter"/>
<module name="WhitespaceAround"/>

<!-- Modifier Checks -->
<module name="ModifierOrder"/>
<module name="RedundantModifier"/>

<!-- Checks for blocks -->
<module name="EmptyBlock"/>
<module name="LeftCurly"/>
<module name="RightCurly"/>
<module name="NeedBraces"/>

<!-- Checks for common coding problems -->
<module name="EmptyStatement"/>
<module name="EqualsHashCode"/>
<module name="IllegalInstantiation"/>
<module name="MissingSwitchDefault"/>
<module name="SimplifyBooleanExpression"/>
<module name="SimplifyBooleanReturn"/>
</module>
</module>
29 changes: 29 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,35 @@
</webResources>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
Copy link
Member

Choose a reason for hiding this comment

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

@harshsennnn indent looks off

<artifactId>maven-checkstyle-plugin</artifactId>
<version>3.3.1</version>
<dependencies>
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
<version>10.12.7</version>
</dependency>
</dependencies>
<configuration>
<configLocation>checkstyle.xml</configLocation>
<encoding>UTF-8</encoding>
<consoleOutput>true</consoleOutput>
<failsOnError>true</failsOnError>
<linkXRef>false</linkXRef>
</configuration>
<executions>
<execution>
<id>validate</id>
<phase>validate</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down