-
Notifications
You must be signed in to change notification settings - Fork 102
Conditionally lock MQG on flutter/flutter
+master
only.
#4685
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
Conversation
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.
Given we only have one merge group (flutter/flutter/master) - this makes perfect sense to not randomly create this check_run.
I double checked the branch protections and master is the only one with MQG.
final lock = await lockMergeGroupChecks(slug, pullRequest.head!.sha!); | ||
final CheckRun? lock; | ||
{ | ||
if (!isFusion) { |
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.
I prefer more positive logic:
if (isFusion && Config.defaultBranch(slug) == pullRequest.head!.ref) {
lock = await _lckMergeGroupChecks(...);
} else {
log.debug('Skipping merge queue guard: not a merge group request; $slug / $ref');
lock = null;
}
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.
Done!
A reason for requesting a revert of flutter/cocoon/4685 could |
reason for revert: Breaks Flutter CI progression from engine -> framework |
…4685)" (#4691) Reverts: #4685 Initiated by: matanlurey Reason for reverting: Breaks Flutter CI progression from engine -> framework Original PR Author: matanlurey Reviewed By: {jtmcdole} This change reverts the following previous change: Closes flutter/flutter#168867.
Closes flutter/flutter#168867.