-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Add required_reviewers support to PullRequestRuleParameters (#3806)
#3811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…solves google#3806) Signed-off-by: Sadique Azmi <[email protected]>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
required_reviewers support to PullRequestRuleParameters (#3806)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3811 +/- ##
=======================================
Coverage 92.27% 92.27%
=======================================
Files 192 192
Lines 13896 13896
=======================================
Hits 12823 12823
Misses 884 884
Partials 189 189 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @smazmi!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29
Got it, thanks @gmlewis ! :) |
| RulesetReviewerTypeTeam RulesetReviewerType = "Team" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User is missing?
| RulesetReviewerTypeTeam RulesetReviewerType = "Team" | |
| ) | |
| RulesetReviewerTypeTeam RulesetReviewerType = "Team" | |
| RulesetReviewerTypeUser RulesetReviewerType = "User" | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review, @alexandear!
I originally added both Team and User reviewer types, but after testing, it looks like GitHub’s API only supports Team for the required_reviewers field.
What I found
-
UI: Only teams can be selected.
-
API tests:
# Team curl -X POST .../rulesets -d '{"reviewer": {"type": "Team", "id": TEAM_ID}}' # 201 Created # User curl -X POST .../rulesets -d '{"reviewer": {"type": "User", "id": USER_ID}}' # 422 Unprocessable Entity (with error: `"data matches no possible input"`)
I tested this a few times, teams worked, users didn't.
I left User out of the enum since it’s not supported yet, but maybe I’m overlooking something.
Should I go ahead and add User back for future compatibility?
Summary
Adds support for GitHub's new ruleset feature that allows requiring specific team approvals on pull requests based on file patterns, as announced in the GitHub changelog on November 3, 2025.
Changes
This PR adds three new types to support the
required_reviewersfield in pull request ruleset parameters:RulesetReviewerType: Type-safe string enum for reviewer typesRulesetRequiredReviewer: Contains minimum approvals, file patterns, and reviewer detailsRulesetReviewer: Nested struct with reviewer ID and typeThe implementation follows existing patterns in the codebase (similar to
BypassActorType) and uses pointer types for proper optionality and JSONomitemptysemantics.Testing
"pull_request_with_required_reviewers"that validates JSON marshaling/unmarshalingAdditional Notes
The
required_reviewersfield is not yet documented in GitHub's OpenAPI schema but is confirmed working in production as of the November 3, 2025 announcement.Resolves #3806
Checklist
script/fmt.shscript/generate.shscript/test.shscript/lint.sh