refactor: move default values to __init__.py #299
Conversation
WalkthroughCentralized default values were moved into module-level constants in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 |
shenxianpeng
added
skip-changelog
and removed
enhancement
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
commit_check/__init__.py (1)
11-11: Critical: Circular import will cause runtime failure.The import order creates a circular dependency:
- Line 11 imports
RuleBuilderfromcommit_check.rule_builderrule_builder.py(lines 6-10) imports these constants fromcommit_check- But at that point,
__init__.pyis stuck at line 11 and hasn't defined the constants yet (lines 24-58)- Result:
ImportError: cannot import name 'DEFAULT_COMMIT_TYPES'Move the
RuleBuilderimport to after the constant definitions:from importlib.metadata import version -from commit_check.rule_builder import RuleBuilder # Exit codes used across the package PASS = 0 FAIL = 1 # ANSI color codes used for CLI output RED = "\033[91m" GREEN = "\033[92m" YELLOW = "\033[93m" RESET_COLOR = "\033[0m" # Follow conventional commits DEFAULT_COMMIT_TYPES = [ "feat", "fix", "docs", "style", "refactor", "test", "chore", "perf", "build", "ci", ] # Follow conventional branch DEFAULT_BRANCH_TYPES = [ "feature", "bugfix", "hotfix", "release", "chore", "feat", "fix", ] # Handle different default values for different rules DEFAULT_BOOLEAN_RULES = { "subject_capitalized": True, "subject_imperative": True, "allow_merge_commits": True, "allow_revert_commits": True, "allow_empty_commits": True, "allow_fixup_commits": True, "allow_wip_commits": True, "require_body": False, "require_signed_off_by": False, } +from commit_check.rule_builder import RuleBuilder + # Default (empty) configuration translated into internal checks structure _rule_builder = RuleBuilder({})Also applies to: 24-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
commit_check/__init__.py(1 hunks)commit_check/rule_builder.py(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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 the set of commit types: build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test
Applied to files:
commit_check/__init__.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 (4)
commit_check/rule_builder.py (4)
6-10: Good centralization of defaults, but depends on fixing circular import.The refactoring to use module-level constants is a good approach for centralizing configuration. However, these imports will fail due to the circular import issue flagged in
commit_check/__init__.py. Once that's resolved by moving theRuleBuilderimport to after the constant definitions, these imports will work correctly.
192-192: LGTM!Good refactoring to use the centralized
DEFAULT_BOOLEAN_RULESdictionary instead of a local defaults dictionary.
217-217: LGTM!Good refactoring to use the module-level
DEFAULT_COMMIT_TYPESconstant. Note thatDEFAULT_COMMIT_TYPESis missing the 'revert' type as flagged in thecommit_check/__init__.pyreview.
222-222: LGTM!Good refactoring to use the module-level
DEFAULT_BRANCH_TYPESconstant.
shenxianpeng
force-pushed
the
feature/move-to-init
branch
from
a4b1a27 to
deaed5d
Compare
github-actions
bot
added
documentation
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 (4)
commit_check/__init__.py(1 hunks)commit_check/rule_builder.py(3 hunks)docs/index.rst(1 hunks)docs/troubleshoot.rst(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/index.rst
🚧 Files skipped from review as they are similar to previous changes (2)
- commit_check/rule_builder.py
- commit_check/init.py
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain Sphinx documentation under docs/ to build HTML docs with sphinx-build
Files:
docs/troubleshoot.rst
| Bypass Specific Hook | ||
| ~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| Use the ``--no-verify`` flag to skip the pre-commit hook: | ||
|
|
||
| .. code-block:: shell | ||
|
|
||
| # Amend the commit without running hooks | ||
| git commit --amend --author="Xianpeng Shen <xianpeng.shen@gmail.com>" --no-edit --no-verify | ||
|
|
||
| Bypass All Hooks | ||
| ---------------- | ||
|
|
||
| Alternatively, use the ``SKIP=your-hook-name`` environment variable, like below: | ||
|
|
||
| .. code-block:: shell | ||
|
|
||
| # Set the correct Git author name | ||
| git config user.name "Xianpeng Shen" | ||
|
|
||
| # Force amend while skipping the specified hook | ||
| SKIP=check-author-name git commit --amend --author="Xianpeng Shen <xianpeng.shen@gmail.com>" --no-edit |
There was a problem hiding this comment.
Fix misleading hook-bypass guidance
git commit --no-verify disables all hooks, while SKIP=… (pre-commit) skips only named hooks. The current headings/text invert that behavior and will mislead users. Please correct the descriptions/headings so they reflect the actual scope of each option.
-Bypass Specific Hook
-~~~~~~~~~~~~~~~~~~~~
-
-Use the ``--no-verify`` flag to skip the pre-commit hook:
+Bypass All Hooks
+----------------
+
+Use the ``--no-verify`` flag to skip every pre-commit and commit-msg hook:
@@
-Bypass All Hooks
-----------------
-
-Alternatively, use the ``SKIP=your-hook-name`` environment variable, like below:
+Skip Selected Hooks
+~~~~~~~~~~~~~~~~~~~
+
+Alternatively, use the ``SKIP=your-hook-name`` environment variable with pre-commit to skip only particular hooks:🤖 Prompt for AI Agents
In docs/troubleshoot.rst around lines 24 to 45 the headings and descriptions
incorrectly state hook scopes: git commit --no-verify actually disables all
hooks while SKIP=your-hook-name only skips the named pre-commit hooks; update
the headings and explanatory text so the "Bypass All Hooks" section describes
--no-verify and its example, and the "Bypass Specific Hook" section describes
SKIP=your-hook-name and its example, adjusting the comments to state that
--no-verify disables all hooks and SKIP skips only the specified hook(s).
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 (4)
commit_check/__init__.py(1 hunks)commit_check/rule_builder.py(3 hunks)docs/index.rst(1 hunks)docs/troubleshoot.rst(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/index.rst
- docs/troubleshoot.rst
🚧 Files skipped from review as they are similar to previous changes (1)
- commit_check/init.py
🔇 Additional comments (3)
commit_check/rule_builder.py (3)
192-192: Enhanced flexibility with per-check default values.The change from a hardcoded
TruetoDEFAULT_BOOLEAN_RULES.get(check, True)allows different boolean checks to have different default values while maintaining backward compatibility with theTruefallback.
222-222: Confirm DEFAULT_BRANCH_TYPES matches legacy defaults.DEFAULT_BRANCH_TYPES is currently defined as
["feature", "bugfix", "hotfix", "release", "chore", "feat", "fix"]. Ensure this exactly reflects the previous hardcoded branch-type list to maintain backward compatibility.
6-10: Ensure default constants preserve previous behavior.Confirm that DEFAULT_COMMIT_TYPES, DEFAULT_BRANCH_TYPES, and DEFAULT_BOOLEAN_RULES in
commit_check/__init__.pyexactly match the hardcoded defaults you removed fromrule_builder.pyto avoid unintended behavior changes.
shenxianpeng
force-pushed
the
feature/move-to-init
branch
from
33597c0 to
2135166
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #299 +/- ##
==========================================
- Coverage 87.33% 86.98% -0.35%
==========================================
Files 8 8
Lines 687 684 -3
==========================================
- Hits 600 595 -5
- Misses 87 89 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #299 will not alter performanceComparing Summary
Footnotes
|
shenxianpeng
removed
documentation
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)
commit_check/__init__.py (1)
1-7: Update module docstring to reflect new exports.The docstring lists exports but doesn't mention the newly added
DEFAULT_COMMIT_TYPES,DEFAULT_BRANCH_TYPES, andDEFAULT_BOOLEAN_RULESconstants.Apply this diff to update the docstring:
"""The commit-check package's base module. Exports: PASS / FAIL exit codes ANSI color constants + DEFAULT_COMMIT_TYPES + DEFAULT_BRANCH_TYPES + DEFAULT_BOOLEAN_RULES __version__ (package version) """
🧹 Nitpick comments (2)
commit_check/__init__.py (2)
21-56: Consider defining__all__for explicit exports.The module doesn't define
__all__, meaning all public symbols (those not starting with_) will be exported byfrom commit_check import *. Consider adding__all__to explicitly control which symbols are exported, improving API clarity and preventing unintended exports if internal helpers are added later.Add this after the imports (around line 10):
__all__ = [ "PASS", "FAIL", "RED", "GREEN", "YELLOW", "RESET_COLOR", "DEFAULT_COMMIT_TYPES", "DEFAULT_BRANCH_TYPES", "DEFAULT_BOOLEAN_RULES", "CONFIG_FILE", "__version__", ]
35-43: Remove or document shorthand branch prefixes in DEFAULT_BRANCH_TYPES.
In commit_check/init.py (lines 35–43),DEFAULT_BRANCH_TYPESincludes both ‘feature’/‘feat’ and ‘bugfix’/‘fix’, but all existing tests and patterns use only the long forms. If shorthand support isn’t required, remove ‘feat’ and ‘fix’.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
commit_check/__init__.py(1 hunks)docs/troubleshoot.rst(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/troubleshoot.rst
🧰 Additional context used
🧠 Learnings (1)
📚 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 the set of commit types: build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test
Applied to files:
commit_check/__init__.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). (11)
- GitHub Check: install (3.14, macos-latest)
- GitHub Check: install (3.9, macos-latest)
- GitHub Check: install (3.13, windows-latest)
- GitHub Check: install (3.14, windows-latest)
- GitHub Check: install (3.14, ubuntu-24.04)
- GitHub Check: install (3.10, windows-latest)
- GitHub Check: install (3.12, windows-latest)
- GitHub Check: install (3.11, windows-latest)
- GitHub Check: install (3.10, macos-latest)
- GitHub Check: install (3.9, ubuntu-24.04)
- GitHub Check: install (3.9, windows-latest)
1 participant
Summary by CodeRabbit
New Features
Refactor
Documentation