fix: update regex to support breaking changes#305
Conversation
WalkthroughUpdated the commit subject validator regex in commit_check/engine.py to support conventional-commit breaking-change syntax (e.g., feat!:, fix(scope)!:). Added two unit tests covering breaking-change cases in tests/engine_test.py. No exported/public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #305 +/- ##
=======================================
Coverage 87.29% 87.29%
=======================================
Files 8 8
Lines 685 685
=======================================
Hits 598 598
Misses 87 87 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #305 will not alter performanceComparing Summary
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)
tests/engine_test.py (1)
782-800: LGTM! Tests adequately cover breaking change scenarios.The two new test cases provide good coverage for the breaking change notation feature:
- Without scope:
feat!:- With scope:
fix(auth)!:Both tests verify that valid imperative verbs still pass validation with the breaking change marker present.
Consider adding a negative test case to verify that breaking change notation with non-imperative verbs still fails:
def test_validate_with_breaking_change_non_imperative(self): """Test validation with breaking change and non-imperative verb.""" rule = ValidationRule(check="imperative") validator = SubjectImperativeValidator(rule) context = ValidationContext(stdin_text="feat!: updated authentication system") with patch("commit_check.util._print_failure"): result = validator.validate(context) assert result == ValidationResult.FAILThis would explicitly confirm that the breaking change syntax doesn't bypass imperative mood validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
commit_check/engine.py(1 hunks)tests/engine_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/engine_test.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: commit-check/commit-check#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
PR: commit-check/commit-check#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
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
PR: commit-check/commit-check#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)
Applied to files:
commit_check/engine.py
🧬 Code graph analysis (1)
tests/engine_test.py (1)
commit_check/engine.py (12)
SubjectImperativeValidator(192-213)ValidationContext(27-32)validate(42-44)validate(100-114)validate(137-145)validate(241-250)validate(287-303)validate(309-329)validate(356-370)validate(393-418)validate(441-456)ValidationResult(19-23)
⏰ 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)
- GitHub Check: install (3.14, ubuntu-24.04)
- GitHub Check: install (3.13, windows-latest)
- GitHub Check: install (3.13, ubuntu-24.04)
- GitHub Check: install (3.13, macos-latest)
- GitHub Check: install (3.14, macos-latest)
- GitHub Check: install (3.14, windows-latest)
- GitHub Check: install (3.11, macos-latest)
- GitHub Check: install (3.12, ubuntu-24.04)
- GitHub Check: install (3.9, windows-latest)
- GitHub Check: install (3.11, windows-latest)
- GitHub Check: install (3.10, ubuntu-24.04)
- GitHub Check: install (3.11, ubuntu-24.04)
- GitHub Check: install (3.12, windows-latest)
- GitHub Check: install (3.12, macos-latest)
- GitHub Check: install (3.9, macos-latest)
- GitHub Check: install (3.9, ubuntu-24.04)
- GitHub Check: install (3.10, windows-latest)
- GitHub Check: Run benchmarks
🔇 Additional comments (1)
commit_check/engine.py (1)
203-204: LGTM! Regex correctly supports breaking change notation.The regex update properly adds support for the conventional commit breaking change indicator (
!) while maintaining backward compatibility. The pattern correctly handles:
feat!: descriptionfix(scope)!: description- All existing formats without
!Based on learnings.
1 participant
closes #301
Summary by CodeRabbit