feat: wildcard conventional-title and conditional merge requirements#1014
feat: wildcard conventional-title and conditional merge requirements#1014
Conversation
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughRefactors merge-requirements assembly in pull_request_handler into a new method and adds wildcard ("*") support for the conventional-title check, plus related schema/example docs and tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Review Summary by QodoSupport wildcard conventional-title and conditional merge requirements
WalkthroughsDescription• Support wildcard "*" for conventional-title config to accept any commit type • Make merge requirements in welcome comment conditional based on configuration • LGTM count shown only when minimum_lgtm > 0 • Verified requirement shown only when verified_job is enabled • Add 8 wildcard validation tests and 7 merge requirements tests Diagramflowchart LR
A["Config: conventional-title"] -->|wildcard '*'| B["Accept any type format"]
A -->|specific types| C["Accept only listed types"]
B --> D["Validate format only"]
C --> D
D --> E["Conventional Commits Check"]
F["Config: minimum_lgtm, verified_job"] --> G["Dynamic merge requirements"]
G --> H["Conditional LGTM requirement"]
G --> I["Conditional Verified requirement"]
H --> J["Welcome comment"]
I --> J
File Changes1. webhook_server/libs/handlers/runner_handler.py
|
Code Review by Qodo
1. Wildcard missing in examples
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webhook_server/libs/handlers/runner_handler.py (1)
475-514:⚠️ Potential issue | 🟡 MinorLOW: Failure guidance is inconsistent in wildcard mode.
When wildcard is configured, the message still says “Type must be one of the configured types” (Line 503), which is inaccurate and user-confusing.
Suggested fix
else: if is_wildcard: types_display = "any valid type (wildcard `*` configured)" + type_rule = "Type can be any valid token (wildcard `*` configured)" else: types_display = ", ".join(f"`{t}`" for t in allowed_names) + type_rule = "Type must be one of the configured types" output["title"] = "❌ Conventional Title" output["summary"] = "Conventional Commit Format Violation" output["text"] = f"""## Conventional Commits Validation Failed @@ **Format rules:** -- Type must be one of the configured types +- {type_rule} - Optional scope in parentheses: `(scope)` - Optional breaking change indicator: `!`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webhook_server/libs/handlers/runner_handler.py` around lines 475 - 514, The failure guidance text incorrectly states "Type must be one of the configured types" even when is_wildcard is true; update the assembly of output["text"] in runner_handler.py (the block that constructs types_display and the message) to conditionally change that rule sentence based on is_wildcard: when is_wildcard use wording like "Type may be any valid type (wildcard `*` configured)" and when not use the existing "Type must be one of the configured types"; ensure the change references the is_wildcard flag and the types_display variable so the rest of the message stays the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webhook_server/libs/handlers/pull_request_handler.py`:
- Around line 509-516: The merge requirement text uses the literal "conflict"
instead of the canonical conflicts label name; update the blockers construction
in pull_request_handler.py so it appends the same label identifier used
elsewhere (e.g., HAS_CONFLICTS_LABEL_STR or the existing constant that
represents "has-conflicts") instead of the hardcoded "conflict", and ensure the
requirements.append call (the line building "**No Blockers**: No {',
'.join(blockers)} labels") reflects that constant so wording matches the rest of
the codebase (refer to blockers list, WIP_STR, HOLD_LABEL_STR, and the
requirements.append call).
In `@webhook_server/tests/test_runner_handler.py`:
- Around line 650-697: The parametrize decorator on
test_conventional_title_wildcard should use a tuple of parameter names instead
of a single string and avoid literal booleans in the param values: change the
decorator first argument to ("title", "should_pass", "reason") and replace the
True/False literals in the data rows with a non-boolean flag (e.g. "pass"/"fail"
or pytest.param with a marker), then update the test signature/consumers of
should_pass (in test_conventional_title_wildcard) to treat should_pass as that
flag (string or enum) and branch on its value instead of relying on boolean
truthiness so it satisfies PT006 and FBT001.
---
Outside diff comments:
In `@webhook_server/libs/handlers/runner_handler.py`:
- Around line 475-514: The failure guidance text incorrectly states "Type must
be one of the configured types" even when is_wildcard is true; update the
assembly of output["text"] in runner_handler.py (the block that constructs
types_display and the message) to conditionally change that rule sentence based
on is_wildcard: when is_wildcard use wording like "Type may be any valid type
(wildcard `*` configured)" and when not use the existing "Type must be one of
the configured types"; ensure the change references the is_wildcard flag and the
types_display variable so the rest of the message stays the same.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
examples/config.yamlwebhook_server/config/schema.yamlwebhook_server/libs/handlers/pull_request_handler.pywebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_pull_request_handler.pywebhook_server/tests/test_runner_handler.py
488e651 to
bbc903a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webhook_server/libs/handlers/pull_request_handler.py`:
- Around line 509-516: The "No Blockers" requirement text is misleading because
it only lists label-based blockers (blockers list built via
labels_handler.is_label_enabled and HAS_CONFLICTS_LABEL_STR) while actual
mergeability also depends on pull_request.mergeable; update the
requirements.append call that builds the "**No Blockers**" message to mention
both label-based blockers and that mergeability (pull_request.mergeable) is
required — e.g., enumerate dynamic blocker labels as before and add an explicit
phrase like "and must be mergeable (no conflicts)" or similar so the UI reflects
both label and mergeability checks.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
examples/config.yamlwebhook_server/config/schema.yamlwebhook_server/libs/handlers/pull_request_handler.pywebhook_server/libs/handlers/runner_handler.pywebhook_server/tests/test_pull_request_handler.pywebhook_server/tests/test_runner_handler.py
- Support '*' wildcard for conventional-title to accept any type while enforcing the Conventional Commits format - Make merge requirements in welcome comment conditional: - LGTM count shown only when minimum_lgtm > 0 - Verified shown only when verified_job is enabled - Numbering adjusts dynamically - Use constants (WIP_STR, HOLD_LABEL_STR, HAS_CONFLICTS_LABEL_STR) in merge requirements instead of hardcoded strings - Conditional failure message in wildcard mode Closes #1013
bbc903a to
c1fd5bd
Compare
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
Summary
"*"wildcard forconventional-titleconfig to accept any commit type while still enforcing the Conventional Commits format (<type>[scope]: description)minimum_lgtm > 0verified_jobis enabledConfig example
Changes
webhook_server/libs/handlers/runner_handler.py- Wildcard"*"support with generic regexwebhook_server/libs/handlers/pull_request_handler.py- Dynamic merge requirements listwebhook_server/config/schema.yaml- Document wildcard in schema descriptionexamples/config.yaml- Add wildcard commentwebhook_server/tests/test_runner_handler.py- 8 wildcard test caseswebhook_server/tests/test_pull_request_handler.py- 7 merge requirements testsTest plan
Closes #1013
Summary by CodeRabbit
New Features
Improvements
Tests