fix: enhance pre-commit workflow for commit message validation #1324
Conversation
- Add commit message validation using conventional-precommit-linter - Update GitHub Actions to latest versions (checkout@v4, setup-python@v4) - Add comprehensive error messages with ESP-IDF specific examples - Remove unnecessary file-based pre-commit hooks for Java/Maven project - Focus workflow on commit message validation only
WalkthroughUpdates the pre-commit GitHub Actions workflow: bumps Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant CO as actions/checkout@v4
participant PY as actions/setup-python@v5
participant Runner as Job Runner
participant Validator as Validate Commit Msgs
participant PC as pre-commit
participant Linter as conventional-precommit-linter
Dev->>GH: push / pull_request
GH->>CO: Checkout repo (fetch-depth: 0)
GH->>PY: Setup Python (@v5)
GH->>Runner: start job
Runner->>Validator: run "Validate commit messages"
alt pull_request
Validator->>GH: determine commits between base..head
else other events
Validator->>GH: use latest commit only
end
loop for each commit
Validator->>Validator: extract message, write temp file
Validator->>PC: invoke commit-msg hook with temp file
PC->>Linter: run conventional-precommit-linter
Linter-->>PC: pass/fail
PC-->>Validator: hook status
end
alt any failures
Validator-->>Runner: output failure details + remediation, exit non-zero
else all pass
Validator-->>Runner: succeed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pre-commit.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pre-commit.yml
18-18: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build_windows
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pre-commit.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: build_macos
- GitHub Check: build_windows
| if [ "${{ github.event_name }}" = "pull_request" ]; then | ||
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | ||
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | ||
| COMMITS=$(git log --oneline $BASE_SHA..$HEAD_SHA) | ||
| else | ||
| COMMITS=$(git log --oneline -1) | ||
| fi |
There was a problem hiding this comment.
Ensure commit collection fails fast when the range is missing.
If git log cannot resolve ${{ github.event.pull_request.head.sha }} (common for forked PRs unless we fetch the head ref), the command exits 128 but still assigns an empty string to COMMITS, so the loop never runs and every commit slips past validation. Please fail fast when the log command errors (and likewise in the non-PR branch) so we never treat “couldn’t fetch commits” as “all commits valid”.
- COMMITS=$(git log --oneline $BASE_SHA..$HEAD_SHA)
+ if ! COMMITS=$(git log --oneline "${BASE_SHA}..${HEAD_SHA}"); then
+ echo "Failed to list commits between ${BASE_SHA} and ${HEAD_SHA}. Did we fetch the head ref?" >&2
+ exit 1
+ fi
@@
- COMMITS=$(git log --oneline -1)
+ if ! COMMITS=$(git log --oneline -1); then
+ echo "Failed to read the latest commit." >&2
+ exit 1
+ 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.
| if [ "${{ github.event_name }}" = "pull_request" ]; then | |
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | |
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | |
| COMMITS=$(git log --oneline $BASE_SHA..$HEAD_SHA) | |
| else | |
| COMMITS=$(git log --oneline -1) | |
| fi | |
| if [ "${{ github.event_name }}" = "pull_request" ]; then | |
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | |
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | |
| if ! COMMITS=$(git log --oneline "${BASE_SHA}..${HEAD_SHA}"); then | |
| echo "Failed to list commits between ${BASE_SHA} and ${HEAD_SHA}. Did we fetch the head ref?" >&2 | |
| exit 1 | |
| fi | |
| else | |
| if ! COMMITS=$(git log --oneline -1); then | |
| echo "Failed to read the latest commit." >&2 | |
| exit 1 | |
| fi | |
| fi |
🤖 Prompt for AI Agents
.github/workflows/pre-commit.yml around lines 27 to 33: the git log commands
that populate COMMITS can fail (exit 128) and leave COMMITS empty which causes
the script to silently accept no commits; change the logic to fail fast when git
log errors by checking the git log exit status and exiting the workflow with a
non-zero code if it fails (or use a shell option like set -e), e.g., run git log
into a temporary variable and if the command returns non-zero, echo an error and
exit 1, otherwise assign COMMITS to that output; apply the same check in both
the pull_request and non-PR branches.
Description
Please include a summary of the change and which issue is fixed.
Fixes # (IEP-XXX)
Type of change
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit