Fix part of #3926: Add support for GitHub merge queues plus a bunch of other CI improvements#6150
Conversation
BenHenning
left a comment
There was a problem hiding this comment.
Quick self-review spot check.
|
PTAL @adhiamboperes. |
Coverage ReportResultsCoverage Analysis: SKIP ⏭️ This PR did not introduce any changes to Kotlin source or test files.
|
|
(Turning off auto-merge because I need to fix the problem with the script preventing PRs from going into review). |
|
@adhiamboperes latest 2 commits should fix the other issue with PRs being moved to draft incorrectly. I'll update the PR description quickly to explain it, as well, and will re-enable auto-merge. |
BenHenning
left a comment
There was a problem hiding this comment.
Quick self-review spot check.
Coverage ReportResultsNumber of files assessed: 9 Passing coverageFiles with passing code coverage
Exempted coverageFiles exempted from coverage
|
adhiamboperes
left a comment
There was a problem hiding this comment.
LGTM overarll. I have replied to your comment, but do feel free to merge the PR.
|
Unassigning @adhiamboperes since they have already approved the PR. |
|
Hi @BenHenning, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks! |
|
Thanks @adhiamboperes! I don't think there's additional work needed here--I responded to your comment and closed that thread. Restarted the cancelled build workflows and keeping auto-merge on. |
Explanation
Fixes part of #3926
This PR introduces support for GitHub merge queues by:
Besides adding support for merge groups, this PR does a bunch of other things:
build_vars.bzlso that we never need to update it again to keep things in sync (Bazel still requires an update).translatewiki-prsbranch since it does not need to use the PR template nor is it prohibited from force pushes (this was missed in the original implementation).todoIssueResolvedCheckwill be skipped which will lead to the other subsequent steps failing to run (because they rely on the workflow beingfailurestatus).\hregex class which isn't supported in native JavaScript).Important: I did not test these changes, and simply running through CI on this PR won't be sufficient. However I'm fairly sure most of these changes won't cause problems and should work without issue except maybe the merge groups (which we will see once merge queues are enabled after this PR is merged and another PR is sent to be merged). We may discover certain dependency upgrades causing problems. If that happens we can fix them forward since it shouldn't really be necessary to revert this PR.
For actual merge queues I am planning to set them up so that PRs are merged 3 times per day (every 8 hours) so try and maximize the chance of more than one PR being merged at once (though I think we average fewer than 1 PRs per day so this probably won't happen). This may be tweaked in the future to be much longer (I'm considering 24 hours) since merge queues actually overall increase the CI burden on the project (and this is causing some fairly substantial issues on Oppia web right now).
Finally, a fairly significant amount of this PR was either authored by Gemini (for the new logic portions) or otherwise helped in investigation (for merge queues and the TODO check bits). The version checks I did myself fully since there wasn't much advantage to an LLM there, and I basically rewrote everything that was suggested in one way or another. The tool was helpful in bouncing ideas mostly, though I will note it caught the potential deep fetch issue for the compute affected targets check for unit tests & code coverage--I likely would have missed that an would have needed to do a follow-up fix once we tried to use merge queues and saw it fail.
Essential Checklist
For UI-specific PRs only
N/A -- This is a CI infrastructure-only change that won't impact the end user experience in the app.