feat: support commit-check without requiring ref in actions/checkout#322
feat: support commit-check without requiring ref in actions/checkout#322shenxianpeng merged 4 commits intomainfrom
ref in actions/checkout#322Conversation
✅ Deploy Preview for commit-check ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughReworked get_branch_name to prefer git output when non-empty, otherwise fall back to environment variables in order: GITHUB_HEAD_REF, GITHUB_REF_NAME, then "HEAD". Added tests covering the fallback paths and precedence. Changes
Sequence DiagramsequenceDiagram
participant Caller
participant get_branch_name
participant git as "git (cmd_output)"
participant env as "os.getenv"
rect rgb(242,247,255)
Note over Caller,git: Primary lookup
Caller->>get_branch_name: call get_branch_name()
get_branch_name->>git: cmd_output(["git","branch","--show-current"])
git-->>get_branch_name: output (non-empty)
get_branch_name-->>Caller: return git output
end
rect rgb(255,249,240)
Note over get_branch_name,env: Fallback when git output empty
Caller->>get_branch_name: call get_branch_name()
get_branch_name->>git: cmd_output(["git","branch","--show-current"])
git-->>get_branch_name: output (empty or error)
get_branch_name->>env: getenv("GITHUB_HEAD_REF")
env-->>get_branch_name: value or None
alt has GITHUB_HEAD_REF
get_branch_name-->>Caller: return GITHUB_HEAD_REF
else
get_branch_name->>env: getenv("GITHUB_REF_NAME")
env-->>get_branch_name: value or None
alt has GITHUB_REF_NAME
get_branch_name-->>Caller: return GITHUB_REF_NAME
else
get_branch_name-->>Caller: return "HEAD"
end
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (18)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #322 +/- ##
==========================================
+ Coverage 87.26% 87.31% +0.05%
==========================================
Files 8 8
Lines 683 686 +3
==========================================
+ Hits 596 599 +3
Misses 87 87 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #322 will not alter performanceComparing Summary
Benchmarks breakdown
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
commit_check/util.py (1)
56-61: This change correctly enables branch detection in GitHub Actions and will help resolve issue #156.The env var fallback chain (GITHUB_HEAD_REF → GITHUB_REF_NAME → "HEAD") is the right approach. The issue occurs when branch name detection fails—validation rules can't trigger without a correct branch name, preventing the exit code mechanism from working. Your change enables that by properly detecting branch names in GitHub Actions environments.
Consider adding documentation for the GitHub Actions-specific behavior:
def get_branch_name() -> str: """Identify current branch name. + + Falls back to GitHub Actions environment variables when git command returns + no output: GITHUB_HEAD_REF (for pull requests) or GITHUB_REF_NAME (for other events). .. note:: With Git 2.22 and above supports `git branch --show-current` Please open an issue at https://github.com/commit-check/commit-check/issues if you encounter any issue. :returns: A `str` describing the current branch name. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
commit_check/util.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Allow special branches: master, main, HEAD, PR-*
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Enforce Conventional Branch prefixes: bugfix/, feature/, release/, hotfix/, task/, chore/
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Allow special branches: master, main, HEAD, PR-*
Applied to files:
commit_check/util.py
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Enforce Conventional Branch prefixes: bugfix/, feature/, release/, hotfix/, task/, chore/
Applied to files:
commit_check/util.py
⏰ 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: install (3.13, windows-latest)
- GitHub Check: install (3.9, windows-latest)
- GitHub Check: Run benchmarks
🔇 Additional comments (1)
commit_check/util.py (1)
8-8: LGTM - Required import added.The
osmodule import is necessary for accessing environment variables.
shenxianpeng
changed the title
ref in actions/checkout
shenxianpeng
added
the
github_actions
shenxianpeng
force-pushed
the
feature/support-github-user-to-get-env
branch
from
7d8b8eb to
6768e32
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/util_test.py (1)
44-82: Good test coverage for the main fallback scenarios.The new tests properly verify environment variable fallbacks when
cmd_outputreturns empty string, coveringGITHUB_HEAD_REF,GITHUB_REF_NAME,HEAD, and priority order.However, consider adding test coverage for the
CalledProcessErrorcase with environment variables. Currently, the existing testtest_get_branch_name_with_exception(lines 29-42) only verifies that an empty string is returned when an exception is raised, but doesn't verify that environment variables are checked in that scenario.Add a test like this to improve coverage:
@pytest.mark.benchmark def test_get_branch_name_exception_with_env_fallback(self, mocker): """Test fallback to environment variables when cmd_output raises exception.""" mocker.patch( "commit_check.util.cmd_output", side_effect=CalledProcessError(1, "git") ) mocker.patch( "commit_check.util.os.getenv", lambda key: "feature-branch" if key == "GITHUB_HEAD_REF" else None, ) assert get_branch_name() == "feature-branch"Note: This test will pass only after applying the fix suggested in
commit_check/util.pylines 56-64.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
commit_check/util.py(2 hunks)tests/util_test.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Files:
tests/util_test.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Support optional scope in commits, e.g., feat(api): add endpoint
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Support breaking change notation with ! (e.g., feat!: breaking change)
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to tests/** : Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Allow special branches: master, main, HEAD, PR-*
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to tests/** : Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Applied to files:
tests/util_test.py
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Allow special branches: master, main, HEAD, PR-*
Applied to files:
commit_check/util.py
🧬 Code graph analysis (1)
tests/util_test.py (1)
commit_check/util.py (1)
get_branch_name(44-64)
⏰ 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). (17)
- GitHub Check: install (3.9, ubuntu-24.04)
- GitHub Check: install (3.14, windows-latest)
- GitHub Check: install (3.13, ubuntu-24.04)
- GitHub Check: install (3.13, macos-latest)
- GitHub Check: install (3.12, windows-latest)
- GitHub Check: install (3.14, ubuntu-24.04)
- GitHub Check: install (3.14, macos-latest)
- GitHub Check: install (3.13, windows-latest)
- GitHub Check: install (3.10, ubuntu-24.04)
- GitHub Check: install (3.11, ubuntu-24.04)
- GitHub Check: install (3.11, windows-latest)
- GitHub Check: install (3.9, windows-latest)
- GitHub Check: install (3.12, macos-latest)
- GitHub Check: install (3.11, macos-latest)
- GitHub Check: install (3.10, macos-latest)
- GitHub Check: install (3.10, windows-latest)
- GitHub Check: Run benchmarks
🔇 Additional comments (1)
commit_check/util.py (1)
8-8: LGTM!The
osimport is correctly added to support the environment variable fallback logic.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
commit_check/util.py (1)
56-65: Excellent: Past review concern fully addressed.The environment variable fallback is now correctly positioned outside the try-except block (lines 60-64), ensuring it executes in both scenarios:
- When
cmd_outputreturns an empty string- When
CalledProcessErroris raised (line 57-58 setsbranch_name = "")The fallback priority (GITHUB_HEAD_REF → GITHUB_REF_NAME → "HEAD") is appropriate for GitHub Actions workflows.
Minor observation: The
cmd_outputfunction (lines 134-148) doesn't usecheck=True, soCalledProcessErrormay not actually be raised in practice. The exception handler is defensive coding and harmless.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
commit_check/util.py(2 hunks)tests/util_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/util_test.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Support optional scope in commits, e.g., feat(api): add endpoint
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Support breaking change notation with ! (e.g., feat!: breaking change)
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Allow special branches: master, main, HEAD, PR-*
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Enforce Conventional Branch prefixes: bugfix/, feature/, release/, hotfix/, task/, chore/
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Allow special branches: master, main, HEAD, PR-*
Applied to files:
commit_check/util.py
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/branch.py : Enforce Conventional Branch prefixes: bugfix/, feature/, release/, hotfix/, task/, chore/
Applied to files:
commit_check/util.py
⏰ 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). (1)
- GitHub Check: Run benchmarks
🔇 Additional comments (1)
commit_check/util.py (1)
8-8: LGTM: Import required for environment variable access.The
osimport is necessary for theos.getenv()calls in the fallback logic.
|
1 participant
closes #323
Summary by CodeRabbit
Bug Fixes
Tests