feat: add conventional-title check to check pr title#666
Conversation
WalkthroughThis pull request introduces a new configuration option, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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 (
|
|
Report bugs in Issues The following are automatically added:
Available user actions:
Supported /retest check runs
Supported labels
|
![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
🧹 Nitpick comments (2)
webhook_server_container/config/schema.yaml (1)
151-152: Improve theconventional-titleschema definition.The property definition is missing the proper schema structure that would make it consistent with other properties in the file. Consider adding type definition and description for better documentation.
- conventional-title: string + conventional-title: + type: string + description: Comma-separated list of conventional prefixes allowed in pull request titleswebhook_server_container/libs/github_api.py (1)
2253-2269: Enhance conventional title validation to match full conventional commit format.The current implementation only checks if the PR title starts with any of the allowed prefixes, but it doesn't verify the full conventional format which typically requires a colon after the prefix (e.g., "feat: Add new feature").
Consider updating the validation to check for the complete format:
- if any([title.startswith(_name) for _name in allowed_names]): + if any([title.startswith(f"{_name}: ") for _name in allowed_names]): self.set_conventional_title_success(output=output)Also, update the error message to be more specific:
- output["text"] = f"pull_request title must starts with allowed title: {', '.join(allowed_names)}" + output["text"] = f"Pull request title must follow the conventional format: '{prefix}: Description' where prefix is one of: {', '.join(allowed_names)}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
example.config.yaml(1 hunks)webhook_server_container/config/schema.yaml(1 hunks)webhook_server_container/libs/github_api.py(6 hunks)webhook_server_container/utils/constants.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-container
- GitHub Check: python-module-install
- GitHub Check: tox
- GitHub Check: pre-commit
🔇 Additional comments (7)
webhook_server_container/utils/constants.py (1)
16-16: LGTM!The new constant follows the naming convention and is appropriately placed in the constants list.
example.config.yaml (1)
96-96: LGTM!The configuration is well-documented with a clear comment explaining the purpose of the conventional title check.
webhook_server_container/libs/github_api.py (5)
47-47: LGTM!The import is correctly added to the alphabetically sorted list of constants.
431-433: LGTM!The implementation correctly retrieves the configuration value using the same pattern as other configuration options.
750-760: LGTM!The status management methods follow the same consistent pattern as other similar methods in the file.
1727-1729: LGTM!The implementation correctly adds the conventional title check to the required status checks when configured.
868-869: LGTM!The implementation correctly integrates the conventional title check into the pull request webhook processing flow.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
webhook_server_container/libs/github_api.py (2)
2253-2269: Complete implementation of conventional title check logic.The implementation correctly validates that PR titles start with one of the allowed prefixes specified in the configuration. The logic is simple and effective.
A couple of potential improvements to consider:
- The check could be more robust by splitting on
:and checking only the prefix part- The allowed prefixes could be trimmed of whitespace for better user experience
- allowed_names = self.conventional_title.split(",") - title = self.pull_request.title - if any([title.startswith(_name) for _name in allowed_names]): + allowed_names = [name.strip() for name in self.conventional_title.split(",")] + title = self.pull_request.title + # Check if title starts with any allowed prefix (optionally followed by ":") + if any([title.startswith(_name) or title.startswith(f"{_name}:") for _name in allowed_names]):
2256-2259: Consider adding more detailed output for successful checks.Currently, the success case doesn't provide any feedback in the output. Consider adding informative text to acknowledge which convention was matched.
output: Dict[str, str] = { "title": "Conventional Title", "summary": "", "text": "", } self.set_conventional_title_in_progress() allowed_names = self.conventional_title.split(",") title = self.pull_request.title if any([title.startswith(_name) for _name in allowed_names]): + output["summary"] = "Success" + output["text"] = f"Pull request title follows the conventional format." self.set_conventional_title_success(output=output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webhook_server_container/libs/github_api.py(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: tox
- GitHub Check: build-container
- GitHub Check: pre-commit
- GitHub Check: python-module-install
🔇 Additional comments (5)
webhook_server_container/libs/github_api.py (5)
47-47: Import of new constant looks good.The constant
CONVENTIONAL_TITLE_STRis properly imported from the constants module.
431-433: Good implementation of configuration retrieval.The code correctly retrieves the
conventional-titleconfiguration using the existingget_value_from_dictshelper function, which handles fallbacks between repository-specific and global configurations.
750-761: Status management methods follow consistent patterns.These new methods for managing the conventional title check status follow the same pattern as other similar status methods in the codebase, which is good for consistency and maintainability.
868-869: Appropriate placement of check initiation.The conventional title check is correctly initiated in the
process_pull_request_webhook_datamethod, which ensures it runs when pull requests are opened or updated.
1727-1729: Correctly added to required status checks.The conventional title check is properly added to the list of required status checks when it's enabled in the configuration, which ensures PRs can't be merged without passing this check.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
webhook_server_container/libs/github_api.py (2)
2267-2268: Improve error message readability.The current error message format may be confusing to users. The
': ,'.join(allowed_names)will produce output likefeat: ,fix: ,docswhich isn't the clearest way to present the allowed prefixes.- output["text"] = f"Pull request title must starts with allowed title: {': ,'.join(allowed_names)}" + output["text"] = f"Pull request title must start with one of the allowed prefixes followed by a colon: {', '.join(allowed_names)}"
2262-2264: Consider adding additional validation for conventional titles.The current implementation only checks if the title starts with an allowed prefix followed by a colon. For a more robust implementation, you might want to consider additional validations such as:
- Ensuring there's content after the prefix and colon (e.g., "feat: add new feature" rather than just "feat:")
- Checking for proper spacing after the colon
- Supporting scopes in parentheses (e.g., "feat(scope): description")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webhook_server_container/libs/github_api.py(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
webhook_server_container/libs/github_api.py (1)
Learnt from: myakove
PR: myk-org/github-webhook-server#666
File: webhook_server_container/libs/github_api.py:2254-2255
Timestamp: 2025-02-25T12:01:42.999Z
Learning: In the ProcessGithubWehook class, the `self.conventional_title` attribute is set during initialization by the `_repo_data_from_config()` method, and doesn't need additional existence checks.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: python-module-install
- GitHub Check: tox
- GitHub Check: pre-commit
- GitHub Check: build-container
🔇 Additional comments (1)
webhook_server_container/libs/github_api.py (1)
2253-2270: The conventional title check implementation looks good.The implementation properly validates PR titles against configured conventional prefixes and follows the same patterns as other checks in the codebase. It clearly reports success or failure depending on whether the PR title starts with an allowed prefix followed by a colon.
|
/verified |
3 participants
Summary by CodeRabbit