-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add missing MergeQueueRuleParameters config properties
#3823
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
Add missing MergeQueueRuleParameters config properties
#3823
Conversation
|
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3823 +/- ##
=======================================
Coverage 92.32% 92.32%
=======================================
Files 194 194
Lines 13979 13979
=======================================
Hits 12906 12906
Misses 884 884
Partials 189 189 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MergeQueueRuleParameters config properties
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, @patrickcarnahan!
Once you make this changes, please run step 4 of CONTRIBUTING.md and push the changes to this PR.
|
|
||
| // MergeQueueRuleParameters represents the merge_queue rule parameters. | ||
| type MergeQueueRuleParameters struct { | ||
| ActorControlledMerging bool `json:"actor_controlled_merging,omitempty"` |
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.
If this is an optional parameter, as indicated by the inclusion of omitempty, then this should be a *bool instead of a bool.
| type MergeQueueRuleParameters struct { | ||
| ActorControlledMerging bool `json:"actor_controlled_merging,omitempty"` | ||
| CheckResponseTimeoutMinutes int `json:"check_response_timeout_minutes"` | ||
| CheckRunRetriesLimit int `json:"check_run_retries_limit,omitempty"` |
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.
Same here. This should be *int instead of int.
|
@gmlewis thanks for the quick turnaround! unfortunately i should have done a little more research first as these properties are not documented nor are they available first repos. for those reasons i think the right thing to do is close this pr out for the time being. thanks again! |
fixes #3822
This adds the missing properties
actor_controlled_mergingandcheck_run_retries_limitto the merge queue rule configuration object. The new properties are added asomitemptyto avoid behavioral changes for existing logic that may not be setting these values.Happy to adjust course and remove
omitemptyand fix tests if that's the desired take.