chore: auto request all reviewers #4093
Conversation
| # NOTE: This assigned the team itself, not members of the team. The Github | ||
| # team auto PR assignment will then turn this into individuals | ||
| - team:celestia-core # This is the Github Team | ||
| - cmwaters |
There was a problem hiding this comment.
@cmwaters LMK if you would like to be removed from this list. If that's the case, we can explicitly add you to PRs that need your attention
There was a problem hiding this comment.
I'm okay with this for now. I'm not too sensitive to github notifications
| # directory owners | ||
| # NOTE: the directory owners should include the global owners unless the global | ||
| # owner is fully deferring ownership to the directory owner | ||
| docs @liamsi @celestiaorg/celestia-core | ||
| specs @liamsi @celestiaorg/celestia-core | ||
| x/blobstream @rach-id @evan-forbes |
There was a problem hiding this comment.
I removed these b/c I think they outlived their purpose
📝 WalkthroughWalkthroughThe pull request includes modifications to two files in the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
![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
🧹 Outside diff range and nitpick comments (1)
.github/auto_request_review.yml (1)
4-8: Consider implementing a more maintainable reviewer configurationWhile the current implementation achieves the goal of requesting all reviewers, it introduces maintenance overhead as the list needs manual updates when team members join or leave. Consider these alternatives:
- Use GitHub team references (e.g.,
org/team-name) to automatically manage membership- Split the configuration into separate groups for core and app engineers for better organization
Example structure:
reviewers: groups: core-engineers: - org/celestia-core-team app-engineers: - org/celestia-app-team defaults: - group: core-engineers - group: app-engineers
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/CODEOWNERS(0 hunks).github/auto_request_review.yml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/CODEOWNERS
🔇 Additional comments (2)
.github/auto_request_review.yml (2)
4-4: Consider adding an opt-out mechanism for @cmwaters
Based on the previous review discussion, @cmwaters might want to opt out of automatic review requests.
4-8: Verify the reviewer assignment functionality in a test environment
The PR description mentions that the implementation is untested. Since this is a critical workflow change affecting all PRs, it should be tested before merging.
Run this script to verify the current team membership and ensure all required reviewers are included:
There wasn't a decision so this is up for debate. Currently we have a rule that each PR must have 2 approvals before being merged. Since 2 random ppl get tagged on each PR, it can take a while for those two people to review. I usually add all core/app engineers as reviewers on my PRs so I figured I'd automate it. |
| # NOTE: This assigned the team itself, not members of the team. The Github | ||
| # team auto PR assignment will then turn this into individuals | ||
| - team:celestia-core # This is the Github Team | ||
| - cmwaters |
There was a problem hiding this comment.
I'm okay with this for now. I'm not too sensitive to github notifications
Closes #4092
Note: hasn't been tested yet. Seems like something we can verify post merge.