-
Notifications
You must be signed in to change notification settings - Fork 36
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
Conversation
Warning Rate limit exceeded@harshsennnn has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughA new GitHub Actions workflow named Changes
Sequence DiagramsequenceDiagram
participant PR as Pull Request
participant GA as GitHub Actions
participant Checkstyle as Checkstyle Validator
participant Repo as Repository
PR->>GA: Trigger workflow
GA->>Repo: Checkout code
GA->>GA: Setup JDK 17
GA->>GA: Identify changed Java files
GA->>Checkstyle: Validate code style
Checkstyle-->>GA: Validation result
alt Style Checks Pass
GA->>PR: Allow merge
else Style Checks Fail
GA->>PR: Block merge
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
checkstyle.xml (1)
18-66
: Consider additional Checkstyle checks for modern Java featuresThe configuration is comprehensive but could benefit from additional checks:
<module name="TreeWalker"> + <!-- Checks for Java 17 features --> + <module name="PatternVariableDeclaration"/> + <module name="RecordDeclaration"/> + + <!-- Additional useful checks --> + <module name="MissingOverride"/> + <module name="FinalClass"/> + <module name="InterfaceIsType"/> + <module name="VisibilityModifier"/> </module>pom.xml (1)
439-466
: Fix plugin configuration indentationThe indentation in the plugin configuration is inconsistent with the rest of the file. Other plugins use tabs, while this one uses spaces:
<plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-checkstyle-plugin</artifactId> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-checkstyle-plugin</artifactId>(Continue fixing indentation for the entire plugin configuration to match the project's style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/checkstyle.yml
(1 hunks)checkstyle.xml
(1 hunks)pom.xml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/checkstyle.yml
[error] 2-2: trailing spaces
(trailing-spaces)
[error] 3-3: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 56-56: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.4)
.github/workflows/checkstyle.yml
18-18: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
35-35: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
53-53: the runner of "actions/github-script@v6" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/checkstyle.yml
Outdated
|
||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update GitHub Actions to their latest versions
Several actions are using outdated versions. Update them to their latest stable versions:
- uses: actions/checkout@v3
+ uses: actions/checkout@v4
- uses: actions/setup-java@v3
+ uses: actions/setup-java@v4
- uses: actions/cache@v3
+ uses: actions/cache@v4
- uses: actions/github-script@v6
+ uses: actions/github-script@v7
Also applies to: 23-23, 35-35, 53-53
🧰 Tools
🪛 actionlint (1.7.4)
18-18: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/checkstyle.yml
Outdated
paths: | ||
- '/*.java' # Monitor all Java files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the path filter pattern for Java files
The current pattern '/*.java'
will only match Java files in the root directory. To monitor all Java files in the repository, use:
paths:
- - '/*.java' # Monitor all Java files
+ - '**/*.java' # Monitor all Java files in any directory
📝 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.
paths: | |
- '/*.java' # Monitor all Java files | |
paths: | |
- '**/*.java' # Monitor all Java files in any directory |
.github/workflows/checkstyle.yml
Outdated
- name: Run Checkstyle on Changed Files | ||
if: steps.changed-files.outputs.any_changed == 'true' | ||
run: | | ||
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do | ||
mvn checkstyle:check -Dcheckstyle.includes=$file | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Optimize Checkstyle execution
Running Checkstyle individually for each file is inefficient. Consider running it once for all changed files:
- name: Run Checkstyle on Changed Files
if: steps.changed-files.outputs.any_changed == 'true'
run: |
- for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
- mvn checkstyle:check -Dcheckstyle.includes=$file
- done
+ mvn checkstyle:check -Dcheckstyle.includes="${{ steps.changed-files.outputs.all_changed_files }}"
📝 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.
- name: Run Checkstyle on Changed Files | |
if: steps.changed-files.outputs.any_changed == 'true' | |
run: | | |
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do | |
mvn checkstyle:check -Dcheckstyle.includes=$file | |
done | |
- name: Run Checkstyle on Changed Files | |
if: steps.changed-files.outputs.any_changed == 'true' | |
run: | | |
mvn checkstyle:check -Dcheckstyle.includes="${{ steps.changed-files.outputs.all_changed_files }}" |
.github/workflows/checkstyle.yml
Outdated
id: changed-files | ||
uses: tj-actions/changed-files@v39 | ||
with: | ||
files: '/*.java' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the path filter pattern in changed-files action
The same path pattern issue exists here. Update it to match all Java files:
- files: '/*.java'
+ files: '**/*.java'
📝 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.
files: '/*.java' | |
files: '**/*.java' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/checkstyle.yml (1)
1-54
: Remove trailing whitespace and add newline at EOF.Several lines contain trailing whitespace, and the file is missing a newline at the end. While these don't affect functionality, they should be fixed for consistency.
Affected lines: 2, 3, 15, 21, 27, 33, 40, 43
🧰 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)
🪛 yamllint (1.35.1)
[error] 2-2: trailing spaces
(trailing-spaces)
[error] 3-3: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/checkstyle.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/checkstyle.yml
46-46: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type {string => string}
(expression)
🪛 yamllint (1.35.1)
.github/workflows/checkstyle.yml
[error] 2-2: trailing spaces
(trailing-spaces)
[error] 3-3: trailing spaces
(trailing-spaces)
[error] 15-15: trailing spaces
(trailing-spaces)
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 33-33: trailing spaces
(trailing-spaces)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
.github/workflows/checkstyle.yml (3)
1-8
: LGTM! Well-configured workflow triggers.The workflow is properly configured to run on pull requests targeting master and develop branches, with correct path filtering for Java files.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 2-2: trailing spaces
(trailing-spaces)
[error] 3-3: trailing spaces
(trailing-spaces)
9-15
: LGTM! Appropriate job configuration with least privilege permissions.The job is correctly configured to run on Ubuntu with minimal required permissions for content reading and PR writing.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 15-15: trailing spaces
(trailing-spaces)
41-43
: Verify Checkstyle configuration existence.The workflow assumes the existence of Checkstyle configuration in the project. Let's verify this.
✅ Verification successful
Checkstyle configuration verified ✓
The necessary Checkstyle components are properly configured:
checkstyle.xml
exists in the repository root- Maven Checkstyle plugin is configured in
pom.xml
with correct version and configuration🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for Checkstyle configuration file and maven plugin configuration # Look for checkstyle.xml echo "Searching for Checkstyle configuration file..." fd "checkstyle.xml" # Check if maven-checkstyle-plugin is configured in pom.xml echo "Checking maven-checkstyle-plugin configuration..." if [ -f "pom.xml" ]; then grep -A 10 "maven-checkstyle-plugin" pom.xml fiLength of output: 691
🧰 Tools
🪛 yamllint (1.35.1)
[error] 43-43: trailing spaces
(trailing-spaces)
.github/workflows/checkstyle.yml
Outdated
path: ~/.m2/repository | ||
key: ${{ runner.os }}-maven-${{ hashFiles('/pom.xml') }} | ||
restore-keys: ${{ runner.os }}-maven |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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 |
.github/workflows/checkstyle.yml
Outdated
- name: Run Checkstyle on Changed Files | ||
if: steps.changed-files.outputs.any_changed == 'true' | ||
run: | | ||
mvn checkstyle:check -Dcheckstyle.includes="${{ steps.changed-files.outputs}}" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Checkstyle execution for changed files.
The current implementation has two issues:
- Incorrect usage of changed-files outputs
- 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.
- 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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
command_output.log
is excluded by!**/*.log
📒 Files selected for processing (2)
.github/workflows/checkstyle.yml
(1 hunks)checkstyle.xml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- checkstyle.xml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/checkstyle.yml
11-11: reusable workflow call "./PSMRI/.github/workflows/checkstyle.yml@main" at "uses" is not following the format "owner/repo/path/to/workflow.yml@ref" nor "./path/to/workflow.yml". see https://docs.github.com/en/actions/learn-github-actions/reusing-workflows for more details
(workflow-call)
🪛 yamllint (1.35.1)
.github/workflows/checkstyle.yml
[error] 13-13: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/checkstyle.yml (1)
3-8
: LGTM! Well-configured workflow triggers.The workflow is properly configured to:
- Run on pull requests targeting the main branches
- Monitor Java files in any directory with the correct path pattern
.github/workflows/checkstyle.yml
Outdated
style-check: | ||
uses: ./PSMRI/.github/workflows/checkstyle.yml@main | ||
with: | ||
java-version: '17' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix workflow reference format and add newline.
- The reusable workflow reference format is incorrect. GitHub Actions requires the format
owner/repo/path/to/workflow.yml@ref
for cross-repository references or./path/to/workflow.yml
for local references. - Add a newline at the end of the file.
Apply this diff to fix the issues:
style-check:
- uses: ./PSMRI/.github/workflows/checkstyle.yml@main
+ uses: PSMRI/HWC-API/.github/workflows/checkstyle.yml@main
with:
java-version: '17'
+
Note: If the workflow file is meant to be in the same repository, use:
- uses: ./PSMRI/.github/workflows/checkstyle.yml@main
+ uses: ./.github/workflows/checkstyle.yml
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.4)
11-11: reusable workflow call "./PSMRI/.github/workflows/checkstyle.yml@main" at "uses" is not following the format "owner/repo/path/to/workflow.yml@ref" nor "./path/to/workflow.yml". see https://docs.github.com/en/actions/learn-github-actions/reusing-workflows for more details
(workflow-call)
🪛 yamllint (1.35.1)
[error] 13-13: no new line character at the end of file
(new-line-at-end-of-file)
command_output.log
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this.
pom.xml
Outdated
@@ -435,6 +435,35 @@ | |||
</webResources> | |||
</configuration> | |||
</plugin> | |||
|
|||
<plugin> | |||
<groupId>org.apache.maven.plugins</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harshsennnn indent looks off
|
This Pull Request includes Github actions workflow configured to run a checkstyle workflow on pr raised and run the check on changed .java files
Summary by CodeRabbit
New Features
Chores