Add approvers and reviewers to welcome comment#722
Conversation
|
""" WalkthroughThe changes refactor the construction of the welcome message in the Changes
Possibly related issues
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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:
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: 1
🔭 Outside diff range comments (1)
webhook_server_container/libs/github_api.py (1)
197-218: 💡 Verification agent🧩 Analysis chain
Renamed property – please verify all call-sites
The property was renamed to
_prepare_retest_welcome_comment.
Double-check that the old nameprepare_retest_wellcome_msgis no longer referenced anywhere in the codebase (tests, helper modules, etc.).
🏁 Script executed:
#!/bin/bash # Search for stale references to the old property name rg -n "prepare_retest_wellcome_msg"Length of output: 295
Stale test references to renamed method – update tests accordingly
The search shows two lingering mentions of the old name in your test suite. Please update them to use the new
_prepare_retest_welcome_commentmethod:
- File:
webhook_server_container/tests/test_prepare_retest_wellcome_msg.py
• Rename the file (and any imports) totest_prepare_retest_welcome_comment.py
• Change all occurrences ofprepare_retest_wellcome_msgto_prepare_retest_welcome_comment
• Update the assertions to call the method (e.g.process_github_webhook._prepare_retest_welcome_comment()) instead of accessing the old propertyAfter these changes, rerun
rg -n "prepare_retest_wellcome_msg"to confirm no references remain.
🧹 Nitpick comments (4)
webhook_server_container/libs/github_api.py (4)
761-765: Welcome-message detection is fragileMatching only the URL (
issue_url_for_welcome_msg in body) may yield false positives if
users paste that link in regular comments.
A safer sentinel (e.g., an HTML comment<!-- github-webhook-welcome -->) would eliminate collisions.
791-795: Avoid redundant re-building of the welcome message
self._prepare_welcome_comment()is called here even though a (correctly timed) call could be cached.
After applying the earlier refactor, just use:welcome_msg = self._prepare_welcome_comment()but decorate
_prepare_welcome_commentwith@functools.cached_property
so it builds once per instance.
2173-2196:_prepare_owners_welcome_commentis currently unusedThe helper builds the collapsible “Approvers and Reviewers” section but is never called, so its
output never reaches the final comment.Either:
- Insert its result into
_prepare_welcome_comment(), or- Remove the dead code to keep the class lean.
2198-2237: Consider integrating the owners section & minor string improvements
- Inject
_prepare_owners_welcome_comment()into the returned string so newcomers immediately see the approvers/reviewers list.- You can drop the leading newline in the f-string to avoid a blank line at the top of the comment.
Example (simplified):
- return f""" -{self.issue_url_for_welcome_msg} + owners_block = self._prepare_owners_welcome_comment() + return f"""{self.issue_url_for_welcome_msg} {owners_block} ... """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webhook_server_container/libs/github_api.py(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-container
- GitHub Check: python-module-install
- GitHub Check: pre-commit
- GitHub Check: tox
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
webhook_server_container/tests/test_prepare_retest_wellcome_comment.py (1)
5-32: 🛠️ Refactor suggestionTest cases need updating for the conventional_title parameter.
The implementation of
_prepare_retest_welcome_comment(from the relevant snippet) now includes support for theconventional_titleparameter, but the test cases don't include this parameter in their parametrization. Consider updating the test parametrization to include this parameter for comprehensive test coverage.@pytest.mark.parametrize( - "tox, build_and_push_container, pypi, pre_commit, expected", + "tox, build_and_push_container, pypi, pre_commit, conventional_title, expected", [ - (False, False, False, False, " * This repository does not support retest actions"), - (True, False, False, False, " * `/retest tox`: Retest tox\n * `/retest all`: Retest all\n"), + (False, False, False, False, False, " * This repository does not support retest actions"), + (True, False, False, False, False, " * `/retest tox`: Retest tox\n * `/retest all`: Retest all\n"), # ... update the other test cases similarly ... + (False, False, False, False, True, " * `/retest conventional-title`: Retest conventional-title\n * `/retest all`: Retest all\n"), ], ) def test_prepare_retest_wellcome_comment( - self, process_github_webhook, tox, build_and_push_container, pypi, pre_commit, expected + self, process_github_webhook, tox, build_and_push_container, pypi, pre_commit, conventional_title, expected ): # ... existing code ... + process_github_webhook.conventional_title = conventional_title
🧹 Nitpick comments (1)
webhook_server_container/tests/test_prepare_retest_wellcome_comment.py (1)
4-42: Consider updating the file name for consistency.The test method and assertions now correctly use "welcome" instead of "wellcome", but the file name still contains the misspelling ("wellcome"). Consider renaming the file to
test_prepare_retest_welcome_comment.pyfor consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webhook_server_container/libs/github_api.py(5 hunks)webhook_server_container/tests/test_prepare_retest_wellcome_comment.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- webhook_server_container/libs/github_api.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
webhook_server_container/tests/test_prepare_retest_wellcome_comment.py (1)
webhook_server_container/libs/github_api.py (1)
_prepare_retest_welcome_comment(196-216)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: tox
- GitHub Check: pre-commit
- GitHub Check: python-module-install
- GitHub Check: build-container
🔇 Additional comments (2)
webhook_server_container/tests/test_prepare_retest_wellcome_comment.py (2)
34-34: Method name updated to match implementation.The test method name has been properly updated from
test_prepare_retest_wellcome_msgtotest_prepare_retest_wellcome_commentto align with the changes in the main implementation.
42-42: Updated assertion to use private method.The assertion now correctly references the renamed private method
_prepare_retest_welcome_commentinstead of the previous public method.
|
/verified |
|
/approve |
3 participants
Summary by CodeRabbit