feat: support additional allow_branch_names in configuration
#336
Conversation
WalkthroughThis PR adds support for user-configurable allowed branch names beyond the conventional branch pattern. Users can now add custom branch names (like "develop" or "staging") via an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
github-actions
bot
added
bug
✅ Deploy Preview for commit-check ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #336 +/- ##
==========================================
+ Coverage 87.17% 87.46% +0.29%
==========================================
Files 8 8
Lines 686 694 +8
==========================================
+ Hits 598 607 +9
+ Misses 88 87 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/engine_test.py (1)
194-201: Add an assertion intest_validate_with_stdin_textThis test currently just calls
validator.validate(context)without asserting the result. To make it a meaningful regression guard, assert the expected outcome, e.g.:result = validator.validate(context) assert result == ValidationResult.PASSRight now it only verifies “no exception raised”.
🧹 Nitpick comments (4)
commit_check/__init__.py (1)
44-45: Check Python version support forlist[str]annotationIf this project still supports Python 3.8 or lower,
list[str]will be invalid syntax at runtime. In that case, either:
- Switch to
from typing import ListandDEFAULT_BRANCH_NAMES: List[str] = [], or- Add
from __future__ import annotationsat the top of the module.Also consider using a tuple for the default (
DEFAULT_BRANCH_NAMES: List[str] = []used only as a default and never mutated, but a tuple or explicit copy when reading can make that intent clearer).commit_check/rules_catalog.py (1)
115-115: Branch rule suggestion text is accurate; consider minor wording tweakThe updated
suggestmessage correctly mentionsallow_branch_namesandignore_authorsas configuration options. To tighten readability, you might slightly rephrase, e.g.:
"Use <type>/<description> with allowed types, or add the branch name to [branch].allow_branch_names in config. To bypass this check entirely for certain authors, configure [branch].ignore_authors."Purely a clarity/wording suggestion; behavior is fine.
tests/engine_test.py (1)
202-260: BranchValidator tests for develop/staging correctly capture new behaviorThe new tests for
develop/staging:
- Correctly mock
has_commitsandget_branch_name.- Use regexes that explicitly include or exclude those branch names to assert PASS/FAIL behavior.
This aligns with the new
allow_branch_names-driven regex behavior inRuleBuilder. If you later refactor the branch regex shape, consider centralizing the pattern (e.g., build it viaRuleBuilderin these tests) to reduce duplication, but it’s not required for this PR.commit_check/rule_builder.py (1)
227-231: _get_allowed_branch_names behavior and DEFAULT_BRANCH_NAMES usage
_get_allowed_branch_namesmirrors the commit/branch type helpers and behaves reasonably:
- Uses
branch_config.get("allow_branch_names", DEFAULT_BRANCH_NAMES)and- Deduplicates while preserving order via
dict.fromkeys.Two optional improvements you might consider:
- If
DEFAULT_BRANCH_NAMESis intended to ever hold non-empty defaults, centralizing the base names there (instead of in_build_conventional_branch_regex) would avoid duplication with docs.- Optionally validate that
allow_branch_namesis a list to catch misconfigured TOML (today a string would be treated as an iterable of characters, similar to_get_allowed_branch_types).Neither is blocking; the current behavior is internally consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
commit_check/__init__.py(1 hunks)commit_check/rule_builder.py(3 hunks)commit_check/rules_catalog.py(1 hunks)docs/configuration.rst(2 hunks)tests/engine_test.py(1 hunks)tests/rule_builder_test.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain Sphinx documentation under docs/ to build HTML docs with sphinx-build
Files:
docs/configuration.rst
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Files:
tests/rule_builder_test.pytests/engine_test.py
🧠 Learnings (9)
📓 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/
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/commit.py : Support the set of commit types: build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test
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
📚 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:
docs/configuration.rstcommit_check/rules_catalog.pycommit_check/__init__.pycommit_check/rule_builder.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:
docs/configuration.rstcommit_check/rules_catalog.pycommit_check/__init__.pycommit_check/rule_builder.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 tests/** : Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Applied to files:
tests/rule_builder_test.pytests/engine_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.yml : Maintain default validation rules for the tool in .commit-check.yml
Applied to files:
commit_check/rules_catalog.pycommit_check/rule_builder.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/commit.py : Support breaking change notation with ! (e.g., feat!: breaking change)
Applied to files:
commit_check/rules_catalog.pycommit_check/rule_builder.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/main.py : Expose CLI flags: --message, --branch, --help, --version, --config, --dry-run; support combining checks
Applied to files:
commit_check/rules_catalog.pycommit_check/__init__.pycommit_check/rule_builder.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/commit.py : Support the set of commit types: build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test
Applied to files:
commit_check/rule_builder.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/commit.py : Enforce Conventional Commits format: type(scope): description
Applied to files:
commit_check/rule_builder.py
🧬 Code graph analysis (1)
tests/rule_builder_test.py (2)
commit_check/rule_builder.py (2)
_build_conventional_branch_rule(121-138)_get_allowed_branch_names(227-230)commit_check/rules_catalog.py (1)
RuleCatalogEntry(8-12)
⏰ 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, windows-latest)
- GitHub Check: install (3.10, windows-latest)
- GitHub Check: install (3.9, windows-latest)
- GitHub Check: install (3.12, ubuntu-24.04)
- GitHub Check: install (3.10, macos-latest)
- GitHub Check: install (3.14, ubuntu-24.04)
- GitHub Check: install (3.11, windows-latest)
- GitHub Check: install (3.13, macos-latest)
- GitHub Check: install (3.13, windows-latest)
- GitHub Check: install (3.9, macos-latest)
- GitHub Check: install (3.11, macos-latest)
- GitHub Check: install (3.10, ubuntu-24.04)
- GitHub Check: install (3.13, ubuntu-24.04)
- GitHub Check: install (3.12, windows-latest)
- GitHub Check: install (3.12, macos-latest)
- GitHub Check: install (3.11, ubuntu-24.04)
- GitHub Check: install (3.9, ubuntu-24.04)
- GitHub Check: Run benchmarks
🔇 Additional comments (3)
docs/configuration.rst (1)
48-48: allow_branch_names documentation is clear and matches implementationThe example config and options table for
allow_branch_namescorrectly describe:
- It as a
list[str]of additional standalone branch names, and- That
master,main,HEAD, andPR-*are always allowed by default.This aligns with the base names used in
RuleBuilder._build_conventional_branch_regex, so the docs and behavior are in sync.Also applies to: 144-148
tests/rule_builder_test.py (1)
159-228: RuleBuilder tests thoroughly coverallow_branch_namesbehaviorThe new tests exercise the key scenarios:
- Default (no
allow_branch_nameskey) uses only base names.- Custom list merges base and custom names into the regex.
- Explicit empty list still keeps base names.
- Duplicates are removed while preserving order via
_get_allowed_branch_names.This gives good confidence in the new branch-name handling logic.
commit_check/rule_builder.py (1)
128-131: Branch regex construction correctly incorporates additional allowed namesThe updated
_build_conventional_branch_rule+_build_conventional_branch_regexnow:
- Use
allowed_typesfor<type>/...branches, and- Combine hard-coded base names (
master,main,HEAD,PR-.+) with configuredallowed_names.This matches the documented behavior (base names always allowed; extra names via
allow_branch_names). The resulting pattern:types_pattern = "|".join(allowed_types) base_names = ["master", "main", "HEAD", "PR-.+"] all_names = base_names + allowed_names names_pattern = ")|(".join(all_names) regex = rf"^({types_pattern})\/.+|({names_pattern})"is consistent with the previous hard-coded regex while making the extra names configurable.
Also applies to: 237-246
There was a problem hiding this comment.
Pull request overview
This PR adds support for configuring additional allowed branch names through a new allow_branch_names configuration option. This enhancement allows users to specify standalone branch names (like "develop" or "staging") that should be accepted alongside the conventional branch type/description format.
Key changes:
- Added
allow_branch_namesconfiguration option with empty list as default - Modified branch validation regex to include both default and custom branch names
- Updated documentation to describe the new configuration option
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| commit_check/init.py | Adds DEFAULT_BRANCH_NAMES constant as empty list |
| commit_check/rule_builder.py | Implements _get_allowed_branch_names method and updates regex builder to include custom branch names |
| commit_check/rules_catalog.py | Updates suggestion message to mention allow_branch_names option |
| docs/configuration.rst | Documents the new allow_branch_names configuration option |
| tests/rule_builder_test.py | Adds comprehensive tests for default, custom, empty, and duplicate branch name scenarios |
| tests/engine_test.py | Adds integration tests validating branch names are properly allowed or rejected |
CodSpeed Performance ReportMerging #336 will degrade performances by 39.93%Comparing Summary
Benchmarks breakdown
Footnotes
|
shenxianpeng
added
enhancement
closes #335
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.