-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat: support new pre-commit hook check-merge-base
#197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes involve the addition of a new pre-commit hook named Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
check-merge-base
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
.pre-commit-hooks.yaml (1)
37-43
: Consider enhancing the hook configuration.A few suggestions to improve the hook:
- Consider adding a
stages
field to explicitly define when this hook should run (like the check-message hook does).- The description could be more specific about how the target branch is determined.
Example enhancement:
- id: check-merge-base name: check merge base - description: requiring to rebase current branch onto target branch + description: requiring current branch to be rebased onto the target branch (specified via --target-branch or default) entry: commit-check args: [--merge-base] pass_filenames: false language: python + stages: [commit, manual]README.rst (2)
35-35
: Fix typo in alternative tools comparisonThere's a typo in the word "puglin" which should be "plugin".
-As an alternative to GitHub Enterprise `Metadata restrictions <https://docs.github.com/en/enterprise-server@3.11/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets#metadata-restrictions>`_ and Bitbucket's paid puglin `Yet Another Commit Checker <https://marketplace.atlassian.com/apps/1211854/yet-another-commit-checker?tab=overview&hosting=datacenter>`_, Commit Check stands out by integrating DevOps principles and Infrastructure as Code (IaC). +As an alternative to GitHub Enterprise `Metadata restrictions <https://docs.github.com/en/enterprise-server@3.11/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets#metadata-restrictions>`_ and Bitbucket's paid plugin `Yet Another Commit Checker <https://marketplace.atlassian.com/apps/1211854/yet-another-commit-checker?tab=overview&hosting=datacenter>`_, Commit Check stands out by integrating DevOps principles and Infrastructure as Code (IaC).
78-78
: Add description for the new hookThe new
check-merge-base
hook is added without any description. Consider adding a brief comment explaining what this hook does, similar to the comment forcheck-message
. This will help users understand its purpose.- id: check-merge-base + - id: check-merge-base # ensures current branch is rebased onto target branch
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.pre-commit-hooks.yaml
(1 hunks)README.rst
(3 hunks)
🔇 Additional comments (2)
.pre-commit-hooks.yaml (1)
37-43
: LGTM! Hook configuration follows established patterns.
The new hook configuration is well-structured and consistent with existing hooks in the file. It properly integrates with the commit-check tool and aligns with the PR objectives.
README.rst (1)
111-111
: LGTM! Command updated to support merge-base check
The Git hooks command has been correctly updated to include the new --merge-base
option.
Let's verify the command-line option is properly implemented:
✅ Verification successful
LGTM! Command updated to support merge-base check
The Git hooks command has been correctly updated to include the new --merge-base
option.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the --merge-base option is implemented in the code
# Search for merge-base option handling in Python files
rg -t py "merge.?base" -A 5
Length of output: 11661
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #197 +/- ##
=======================================
Coverage 97.27% 97.27%
=======================================
Files 7 7
Lines 294 294
=======================================
Hits 286 286
Misses 8 8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.pre-commit-config.yaml (1)
39-42
: LGTM! New hooks are properly configuredThe new hooks (
check-author-name
,check-author-email
, andcheck-merge-base
) are:
- Correctly formatted and consistent with existing patterns
- Commented out by default, following the established convention
- Properly aligned with the PR objectives for adding merge-base functionality
Consider adding a comment explaining what the
check-merge-base
hook does, similar to how other hooks in the file have explanatory comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.pre-commit-config.yaml
(1 hunks).pre-commit-hooks.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .pre-commit-hooks.yaml
🔇 Additional comments (1)
.pre-commit-config.yaml (1)
35-35
: Verify the stability of commit-check v0.9.1
Let's ensure this version is stable and contains the new hooks mentioned in the PR.
✅ Verification successful
commit-check v0.9.1 is stable and free of known issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the commit-check version and its changelog
# Check the latest version and tags
gh api repos/commit-check/commit-check/tags --jq '.[0:3] | .[] | {name, commit: .commit.sha}'
# Check for any reported issues with v0.9.1
gh api search/issues -X GET -f q="repo:commit-check/commit-check is:issue label:bug v0.9.1"
Length of output: 455
closes #195
Summary by CodeRabbit
New Features
check-merge-base
, to ensure branches are rebased onto the target branch during commits.check-author-name
andcheck-author-email
, which can be enabled if needed.Documentation
README.rst
to include the newcheck-merge-base
hook and modified command usage for clarity and consistency.