chore(commitlint): add commit-msg hook and workflow validation#143
Open
DurgaPrasad-54 wants to merge 2 commits intoPSMRI:mainfrom
Open
chore(commitlint): add commit-msg hook and workflow validation#143DurgaPrasad-54 wants to merge 2 commits intoPSMRI:mainfrom
DurgaPrasad-54 wants to merge 2 commits intoPSMRI:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMigrates commit validation from Husky to a custom Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (local)
participant Git as Git
participant Hook as .git-hooks/commit-msg
participant CL as Commitlint (npx)
Dev->>Git: git commit
Git->>Hook: invoke commit-msg hook with $1
Hook->>CL: npx --yes `@commitlint/cli`@20.4.3 --edit "$1"
CL-->>Hook: exit status
Hook-->>Git: allow/deny commit
Git-->>Dev: success or abort
sequenceDiagram
participant PR as Pull Request (GitHub)
participant Actions as GitHub Actions (commit-check)
participant Repo as Repository (commits)
participant CL as Commitlint (npx)
PR->>Actions: trigger workflow
Actions->>Repo: fetch commits between base..head
loop for each non-merge commit
Actions->>Actions: write commit message to temp file
Actions->>CL: npx --yes `@commitlint/cli`@20.4.3 --config ... --edit temp_file
CL-->>Actions: result
Actions->>Actions: delete temp file
end
Actions-->>PR: pass/fail status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/commitlint.yml (1)
23-24: Handle case when PR has no commits in the range.If
github.event.pull_request.base.shaequalshead.shaor the range is invalid (e.g., force-push scenarios),$commitsmay be empty. With an empty variable, theforloop is skipped silently, which is acceptable, butgit logcould fail with an error if the base SHA is unavailable (e.g., shallow clone edge cases despitefetch-depth: 0).Consider adding a guard:
🛡️ Optional: add a commits check
commits=$(git log --no-merges --pretty=format:"%H" "${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }}") + if [ -z "$commits" ]; then + echo "No commits to validate" + exit 0 + fi for sha in $commits; do🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/commitlint.yml around lines 23 - 24, The script currently runs git log into the commits variable and then iterates with for sha in $commits, but git log can fail or produce an empty string when the PR range is invalid or base/head SHAs are unavailable; add a guard around the git log and the for loop: first validate the PR SHAs (github.event.pull_request.base.sha and head.sha) and/or protect the git log call (e.g., test git rev-parse for the base SHA or capture git log exit status) and if git log fails or returns empty set commits to an empty value, then check if commits is non-empty before entering the for sha in $commits loop so the script handles shallow/invalid-range cases gracefully.README.md (1)
47-49: Consider updating Node.js version requirement.The prerequisites list Node.js v14, but v14 reached end-of-life in April 2023. The CI workflow uses Node.js 20. Consider updating the minimum version to align with the CI environment and current LTS versions.
📝 Suggested update
### Prerequisites -- Node.js (v14 or later) +- Node.js (v20 or later recommended) - npm (comes with Node.js)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 47 - 49, Update the "Prerequisites" section in README.md where it currently lists "Node.js (v14 or later)" to a modern minimum that matches CI and current LTS (e.g. "Node.js (v18 or later)" or "Node.js (v20 or later)"); change the bullet "Node.js (v14 or later)" to the chosen newer minimum and optionally note the CI uses Node.js 20 to keep docs aligned with the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.git-hooks/commit-msg:
- Around line 1-4: The commit-msg hook script contains a valid shell shebang but
lacks the executable bit, so make the hook executable (e.g., run chmod +x on the
commit-msg hook file) so Git can invoke the script that runs the npx --yes
`@commitlint/cli`@20.4.3 --edit "$1" command; ensure the file retains the
#!/bin/sh shebang and execute permission.
In @.github/workflows/commitlint.yml:
- Around line 22-29: The temp file may remain if commitlint fails because the
script exits before rm "$tmp"; add a cleanup function and trap to always remove
the temp file: define a cleanup() that tests [ -n "$tmp" ] && rm -f "$tmp" and
register it with trap 'cleanup INT TERM EXIT' before creating tmp (or at top of
the loop), then proceed to create tmp, run npx --yes `@commitlint/cli`@20.4.3
--edit "$tmp", and after successful removal unset tmp (tmp=) so the trap won’t
try to delete an already-removed file; reference variables/commands commits,
tmp, npx --yes `@commitlint/cli`@20.4.3 --edit and the for sha loop when making
the change.
---
Nitpick comments:
In @.github/workflows/commitlint.yml:
- Around line 23-24: The script currently runs git log into the commits variable
and then iterates with for sha in $commits, but git log can fail or produce an
empty string when the PR range is invalid or base/head SHAs are unavailable; add
a guard around the git log and the for loop: first validate the PR SHAs
(github.event.pull_request.base.sha and head.sha) and/or protect the git log
call (e.g., test git rev-parse for the base SHA or capture git log exit status)
and if git log fails or returns empty set commits to an empty value, then check
if commits is non-empty before entering the for sha in $commits loop so the
script handles shallow/invalid-range cases gracefully.
In `@README.md`:
- Around line 47-49: Update the "Prerequisites" section in README.md where it
currently lists "Node.js (v14 or later)" to a modern minimum that matches CI and
current LTS (e.g. "Node.js (v18 or later)" or "Node.js (v20 or later)"); change
the bullet "Node.js (v14 or later)" to the chosen newer minimum and optionally
note the CI uses Node.js 20 to keep docs aligned with the workflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ff6e00e-ebac-4c22-8c00-938d9ecc6888
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.git-hooks/commit-msg.github/workflows/commitlint.yml.husky/commit-msgREADME.mdcommitlint.config.jspackage.json
💤 Files with no reviewable changes (2)
- .husky/commit-msg
- package.json
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📋 Description
JIRA ID:
Add the commit message validation setup by defining the commit rules directly in commitlint.config.js instead of extending @commitlint/config-conventional.
✅ Type of Change
Summary by CodeRabbit
Documentation
Chores