feat: add configurable PR size labels with custom names, thresholds and colors#829
feat: add configurable PR size labels with custom names, thresholds and colors#829
Conversation
WalkthroughThis change introduces configurable pull request (PR) size labels with customizable thresholds and colors, allowing both global and per-repository overrides via configuration files. The logic for determining PR size labels and their colors has been centralized and extended, with documentation, schema, and tests updated accordingly. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 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. |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webhook_server/libs/labels_handler.py(1 hunks)webhook_server/utils/constants.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: myakove
PR: myk-org/github-webhook-server#586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In `webhook_server_container/libs/github_api.py`, the indentation style used in the `set_pull_request_automerge` method is acceptable as per the project's coding standards.
webhook_server/libs/labels_handler.py (3)
Learnt from: myakove
PR: #586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In webhook_server_container/libs/github_api.py, the indentation style used in the set_pull_request_automerge method is acceptable as per the project's coding standards.
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:925-926
Timestamp: 2024-10-29T10:42:50.163Z
Learning: In webhook_server_container/libs/github_api.py, the method self._keep_approved_by_approvers_after_rebase() must be called after removing labels when synchronizing a pull request. Therefore, it should be placed outside the ThreadPoolExecutor to ensure it runs sequentially after label removal.
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In webhook_server_container/libs/github_api.py, when the function _keep_approved_by_approvers_after_rebase is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
🔇 Additional comments (1)
webhook_server/utils/constants.py (1)
59-59: LGTM!The addition of the new
"size/OMG"label with the appropriate color code follows the existing pattern and correctly supports the extended size classification system.
…and colors
This feature allows repository administrators to define custom PR size
categorization with flexible naming, threshold values, and colors.
Key Features:
- Custom label names (e.g., Tiny, Express, Premium instead of XS, S, M)
- Configurable threshold values for lines changed (additions + deletions)
- CSS3 color name support with automatic hex conversion via webcolors
- 3-level configuration hierarchy (local > repository > global)
- Real-time configuration updates without server restart
- Fallback to static defaults when no custom config provided
- Comprehensive test coverage for all functionality
Technical Changes:
- Added pr-size-thresholds schema validation for both global and repository levels
- Implemented dynamic threshold processing with automatic sorting
- Added webcolors dependency for CSS3 color name to hex conversion
- Fixed type annotations to handle float('inf') with int | float union type
- Converted all configuration keys to kebab-case for consistency
- Updated existing tests to properly mock config.get_value return values
- Added comprehensive test suite for configuration validation and functionality
Documentation Updates:
- Added configurable PR size labels section to README with examples
- Updated example configuration files with pr-size-thresholds examples
- Added configuration naming standards to decisions.md (DEC-004)
- Added commit message standards to decisions.md (DEC-005)
- Updated repository-level override examples
Breaking Changes: None - fully backward compatible with existing static labels
myakove
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
325-383: Excellent detailed feature documentation!The comprehensive subsection covers all important aspects:
- Configuration syntax and rules
- Supported CSS3 color names with examples
- Hierarchy and fallback behavior
- Real-time update semantics
This provides users with all necessary information to implement the feature effectively.
Consider breaking the longer lines to improve readability:
-The webhook server supports configurable pull request size labels with custom names, thresholds, and colors. This feature allows repository administrators to define their own categorization system. +The webhook server supports configurable pull request size labels with custom names, +thresholds, and colors. This feature allows repository administrators to define +their own categorization system.-Configuration changes take effect immediately without server restart. The webhook server re-reads configuration for each incoming webhook event. +Configuration changes take effect immediately without server restart. The webhook +server re-reads configuration for each incoming webhook event.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.gitignore(1 hunks)README.md(5 hunks)examples/.github-webhook-server.yaml(1 hunks)examples/config.yaml(2 hunks)pyproject.toml(1 hunks)webhook_server/config/schema.yaml(2 hunks)webhook_server/libs/labels_handler.py(4 hunks)webhook_server/tests/test_config_schema.py(1 hunks)webhook_server/tests/test_labels_handler.py(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- pyproject.toml
- .gitignore
- examples/.github-webhook-server.yaml
- examples/config.yaml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: myakove
PR: myk-org/github-webhook-server#586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In `webhook_server_container/libs/github_api.py`, the indentation style used in the `set_pull_request_automerge` method is acceptable as per the project's coding standards.
Learnt from: myakove
PR: myk-org/github-webhook-server#612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
webhook_server/libs/labels_handler.py (5)
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In webhook_server_container/libs/github_api.py, when the function _keep_approved_by_approvers_after_rebase is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:925-926
Timestamp: 2024-10-29T10:42:50.163Z
Learning: In webhook_server_container/libs/github_api.py, the method self._keep_approved_by_approvers_after_rebase() must be called after removing labels when synchronizing a pull request. Therefore, it should be placed outside the ThreadPoolExecutor to ensure it runs sequentially after label removal.
Learnt from: myakove
PR: #586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In webhook_server_container/libs/github_api.py, the indentation style used in the set_pull_request_automerge method is acceptable as per the project's coding standards.
Learnt from: myakove
PR: #778
File: webhook_server/libs/pull_request_handler.py:327-330
Timestamp: 2025-05-13T12:06:27.297Z
Learning: In the GitHub webhook server, synchronous GitHub API calls (like create_issue_comment, add_to_assignees, etc.) in async methods should be awaited using asyncio.to_thread or loop.run_in_executor to prevent blocking the event loop.
Learnt from: myakove
PR: #778
File: webhook_server/libs/pull_request_handler.py:327-330
Timestamp: 2025-05-13T12:06:27.297Z
Learning: In the GitHub webhook server, synchronous GitHub API calls (like create_issue_comment, add_to_assignees, etc.) in async methods should be awaited using asyncio.to_thread or loop.run_in_executor to prevent blocking the event loop.
webhook_server/tests/test_labels_handler.py (5)
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In webhook_server_container/libs/github_api.py, when the function _keep_approved_by_approvers_after_rebase is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:925-926
Timestamp: 2024-10-29T10:42:50.163Z
Learning: In webhook_server_container/libs/github_api.py, the method self._keep_approved_by_approvers_after_rebase() must be called after removing labels when synchronizing a pull request. Therefore, it should be placed outside the ThreadPoolExecutor to ensure it runs sequentially after label removal.
Learnt from: myakove
PR: #586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In webhook_server_container/libs/github_api.py, the indentation style used in the set_pull_request_automerge method is acceptable as per the project's coding standards.
Learnt from: myakove
PR: #598
File: webhook_server_container/libs/github_api.py:1860-1874
Timestamp: 2024-10-15T10:37:45.791Z
Learning: In the process_retest_command method in webhook_server_container/libs/github_api.py, _target_tests is defined before use.
Learnt from: myakove
PR: #631
File: webhook_server_container/tests/test_github_api.py:321-327
Timestamp: 2024-11-26T14:30:22.906Z
Learning: In webhook_server_container/tests/test_github_api.py, when using pytest.mark.parametrize with indirect parameters, the pytest.param function may require nested lists to match the expected input structure.
webhook_server/tests/test_config_schema.py (1)
Learnt from: myakove
PR: #598
File: webhook_server_container/libs/github_api.py:1860-1874
Timestamp: 2024-10-15T10:37:45.791Z
Learning: In the process_retest_command method in webhook_server_container/libs/github_api.py, _target_tests is defined before use.
README.md (3)
Learnt from: myakove
PR: #624
File: webhook_server_container/libs/github_api.py:596-604
Timestamp: 2024-11-21T13:34:45.218Z
Learning: In the codebase, aggregation of reviewers and approvers from all OWNERS files is handled within the assign_reviewers method.
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In webhook_server_container/libs/github_api.py, when the function _keep_approved_by_approvers_after_rebase is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
Learnt from: myakove
PR: #586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In webhook_server_container/libs/github_api.py, the indentation style used in the set_pull_request_automerge method is acceptable as per the project's coding standards.
🪛 markdownlint-cli2 (0.17.2)
README.md
327-327: Line length
Expected: 120; Actual: 197
(MD013, line-length)
374-374: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
382-382: Line length
Expected: 120; Actual: 144
(MD013, line-length)
🔇 Additional comments (26)
webhook_server/config/schema.yaml (2)
70-86: LGTM! Well-structured schema for configurable PR size thresholds.The schema correctly defines the new
pr-size-thresholdsconfiguration with appropriate validation rules:
- Required positive integer threshold
- Optional color string for CSS3 color names
- Strict additional properties enforcement
247-262: LGTM! Repository-level override support is properly implemented.The repository-specific
pr-size-thresholdsconfiguration mirrors the global structure, enabling per-repository customization as documented.README.md (4)
69-69: Excellent feature documentation addition!Clear and concise description of the configurable PR size labels feature in the features list.
237-251: LGTM! Comprehensive global configuration example.The global PR size thresholds example clearly demonstrates the configuration syntax with appropriate threshold values and CSS3 color names.
296-307: LGTM! Repository-specific override example is well-structured.The repository-level configuration example effectively shows how to override global settings with custom size categories.
403-414: LGTM! Repository-level example enhances the documentation.The simple repository configuration example effectively demonstrates how to define custom PR size labels at the repository level.
webhook_server/tests/test_labels_handler.py (7)
49-52: Good test setup enhancement for backward compatibility.The configuration mock ensures existing tests continue to use static defaults while new tests can override this behavior.
731-750: LGTM! Comprehensive test for custom PR size thresholds.Test properly validates:
- Configuration parsing from webhook config
- Color conversion to hex values
- Expected sorted threshold structure
751-770: LGTM! Important fallback behavior test.Ensures the system gracefully falls back to static defaults when no custom configuration is provided.
771-806: LGTM! Excellent edge case coverage.Tests handle missing and invalid color configurations with appropriate fallbacks to default colors.
807-856: LGTM! Thorough testing of size calculation with custom thresholds.Tests validate:
- Custom threshold application in size calculation
- Boundary conditions
- Single threshold scenarios
857-876: LGTM! Important sorting validation test.Ensures custom thresholds are properly sorted by threshold value regardless of configuration order.
878-935: LGTM! Comprehensive label color resolution tests.Excellent coverage of the color resolution logic:
- Custom size label colors
- Static label fallback
- Dynamic label handling
- Default color fallback for unknown labels
webhook_server/libs/labels_handler.py (7)
4-4: LGTM! Appropriate dependency addition.The
webcolorslibrary is the standard choice for CSS3 color name to hex conversion.
72-76: LGTM! Clear logic separation for static labels.Good separation of static label handling before dynamic label processing.
78-80: LGTM! Centralized color resolution.The centralized
_get_label_colormethod provides a clean abstraction for color determination.
107-136: LGTM! Comprehensive color resolution logic.Excellent implementation that handles:
- Custom size label colors from configuration
- Static label fallback for backward compatibility
- Dynamic label color mapping
- Default color fallback
137-150: LGTM! Robust color conversion with fallback.Good error handling for invalid color names with multiple fallback levels:
- Primary color name conversion
- Default color name fallback
- Hardcoded hex fallback
151-191: LGTM! Well-implemented custom threshold parsing.Excellent implementation with:
- Proper configuration validation
- Color conversion with fallback
- Sorted threshold ordering
- Graceful fallback to static defaults
The recursive call for fallback is handled correctly to avoid infinite loops.
201-216: LGTM! Improved size calculation logic.The updated
get_sizemethod correctly:
- Uses dynamic thresholds from configuration
- Handles edge cases with proper fallbacks
- Maintains backward compatibility
This resolves the previous size classification logic gap mentioned in past reviews.
webhook_server/tests/test_config_schema.py (6)
510-532: LGTM! Well-structured test for valid PR size thresholds configuration.The test properly validates that the new
pr-size-thresholdsconfiguration can be loaded with both threshold integers and color strings, with appropriate cleanup.
534-557: LGTM! Comprehensive test for repository-level PR size thresholds.The test properly validates that PR size thresholds can be configured at the repository level, which provides the necessary flexibility for different repositories to have different size categorizations.
559-584: LGTM! Thorough test for CSS3 color name validation.The test comprehensively validates that various CSS3 color names are accepted in the PR size thresholds configuration. The incremental threshold values represent a realistic progression of PR size categories.
586-623: LGTM! Important edge case testing for missing fields.The test properly validates the handling of missing required (threshold) and optional (color) fields. The approach of allowing configuration loading with runtime validation is appropriate and well-tested here.
625-680: LGTM! Comprehensive negative testing for invalid threshold values.The test thoroughly covers invalid threshold scenarios including negative values, zero values, and type mismatches. The approach of allowing configuration loading with deferred runtime validation is consistent and well-implemented.
682-697: LGTM! Important edge case test for empty configuration.The test ensures that empty PR size thresholds configuration is handled gracefully, which is crucial for backward compatibility and robustness.
Break up longer lines in the configurable PR size labels documentation to improve readability while maintaining comprehensive coverage of: - Configuration syntax and rules - Supported CSS3 color names with examples - Hierarchy and fallback behavior - Real-time update semantics
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
237-251: Clarify threshold semantics (inclusive / exclusive).Readers may wonder whether the
thresholdvalue is:
- “≥ threshold and < next-threshold”, or
- “> previous-threshold and ≤ threshold”.
A one-sentence clarification after the YAML block avoids ambiguity.
296-307: Duplicate example is helpful but could reference global block to reduce length.Consider replacing the full YAML with: “See global example above; values override at repository level”, plus the shorter three-label snippet. Keeps the README lean.
376-383: Fix markdown-lint MD032 – add blank line before list.
markdownlint-cli2flags the list starting at Line 380 because it is not preceded by a blank line.-Any valid CSS3 color name is supported, including: -- Basic colors: `red`, `green`, `blue`, `orange`, `yellow`, `purple` +Any valid CSS3 color name is supported, including: + +- Basic colors: `red`, `green`, `blue`, `orange`, `yellow`, `purple`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: myakove
PR: myk-org/github-webhook-server#586
File: webhook_server_container/libs/github_api.py:1947-1956
Timestamp: 2024-10-08T09:19:56.185Z
Learning: In `webhook_server_container/libs/github_api.py`, the indentation style used in the `set_pull_request_automerge` method is acceptable as per the project's coding standards.
Learnt from: myakove
PR: myk-org/github-webhook-server#612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In `webhook_server_container/libs/github_api.py`, when the function `_keep_approved_by_approvers_after_rebase` is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
README.md (2)
Learnt from: myakove
PR: #624
File: webhook_server_container/libs/github_api.py:596-604
Timestamp: 2024-11-21T13:34:45.218Z
Learning: In the codebase, aggregation of reviewers and approvers from all OWNERS files is handled within the assign_reviewers method.
Learnt from: myakove
PR: #612
File: webhook_server_container/libs/github_api.py:2089-2100
Timestamp: 2024-10-29T08:09:57.157Z
Learning: In webhook_server_container/libs/github_api.py, when the function _keep_approved_by_approvers_after_rebase is called, existing approval labels have already been cleared after pushing new changes, so there's no need to check for existing approvals within this function.
🪛 markdownlint-cli2 (0.17.2)
README.md
379-379: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🔇 Additional comments (1)
README.md (1)
69-70: Nice addition – new feature bullet reads well.
No issues spotted.
- Add threshold semantics clarification: PRs with changes ≥ threshold and < next-threshold get that label - Reduce duplicate YAML example by referencing global configuration - Fix markdown-lint MD032 by adding blank line before list - Keep README lean while maintaining comprehensive information
|
/verified |
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
2 participants
Summary by CodeRabbit
New Features
Documentation
Configuration
Bug Fixes
Tests
Chores