feat: support additional `allow_branch_names` in configuration by shenxianpeng · Pull Request #336 · commit-check/commit-check · GitHub

closes #335

Summary by CodeRabbit

New Features
- Added configuration option to specify additional custom branch names allowed when using conventional branch validation rules.
Documentation
- Updated configuration documentation with details and examples for the new branch name allowlist feature.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

This 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 allow_branch_names configuration option, which are incorporated into the validation regex pattern during rule building.

Changes

Cohort / File(s)	Summary
Core functionality commit_check/__init__.py, commit_check/rule_builder.py	Added DEFAULT_BRANCH_NAMES constant export; introduced _get_allowed_branch_names() method to deduplicate and preserve order; updated _build_conventional_branch_regex signature to accept allowed_names parameter; modified _build_conventional_branch_rule to retrieve and pass allowed branch names to regex builder.
Configuration & suggestions commit_check/rules_catalog.py, docs/configuration.rst	Updated BRANCH_RULES suggestion text to mention allow_branch_names config option; added allow_branch_names documentation as an optional list field with default empty list under branch options.
Test coverage tests/engine_test.py	Added tests validating that custom branches (develop, staging) are allowed when included in the validation regex and blocked when excluded.
Rule builder tests tests/rule_builder_test.py	Added comprehensive test suite covering default branch names, custom branch names, empty lists, and deduplication behavior of allow_branch_names.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Regex construction logic: Verify that _build_conventional_branch_regex correctly combines default names (master, main, HEAD, PR-.+) with custom allowed_names and produces valid regex patterns
Deduplication method: Ensure _get_allowed_branch_names properly handles duplicates while preserving insertion order
Test assertions: Confirm all test cases in rule_builder_test.py accurately validate regex construction and that engine_test.py covers the full allowed/blocked branch scenarios

Possibly related PRs

refactor: move default values to __init__.py #299: Related to introduction of module-level constants (DEFAULT_BRANCH_TYPES) and updating rule_builder to import/use them; establishes precedent for this PR's approach with DEFAULT_BRANCH_NAMES
chore: add more test cases #294: Related branch validation test coverage expansion in tests/engine_test.py and BranchValidator behavior testing

Suggested labels

developer

Poem

A rabbit hops through branches wide,
"Develop" now may be our guide!
With "staging" too, no more we fret,
Conventional rules with flexibility yet! 🐰✨
Custom names in config set,
The branch validation debt is met!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: adding support for an allow_branch_names configuration option.
Linked Issues check	✅ Passed	The PR fully implements the objective from issue #335 by adding a new allow_branch_names configuration option that allows users to specify additional branch names (like develop) that should pass the branch check, directly addressing the reporter's need.
Out of Scope Changes check	✅ Passed	All changes are focused on implementing the allow_branch_names feature across configuration, rule building, validation, and tests with no unrelated modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/issue-335

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

✅ Deploy Preview for commit-check ready!

Name	Link
🔨 Latest commit	4d84083
🔍 Latest deploy log	https://app.netlify.com/projects/commit-check/deploys/694058905e49280008679f86
😎 Deploy Preview	https://deploy-preview-336--commit-check.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Quality Gate passed

Issues
0 New issues
4 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.46%. Comparing base (996758d) to head (4d84083).
⚠️ Report is 1 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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 in test_validate_with_stdin_text

This 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.PASS
Right now it only verifies “no exception raised”.

🧹 Nitpick comments (4)

commit_check/__init__.py (1)

44-45: Check Python version support for list[str] annotation

If 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 List and DEFAULT_BRANCH_NAMES: List[str] = [], or

Add from __future__ import annotations at 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 tweak

The updated suggest message correctly mentions allow_branch_names and ignore_authors as 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 behavior

The new tests for develop/staging:

Correctly mock has_commits and get_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 in RuleBuilder. If you later refactor the branch regex shape, consider centralizing the pattern (e.g., build it via RuleBuilder in 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_names mirrors 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_NAMES is 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_names is 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

📥 Commits

Reviewing files that changed from the base of the PR and between 996758d and 4d84083.

📒 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.py
tests/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