Move CI jobs for draft PRs to contributor's personal forks of Mill
This PR aims to reduce the bottleneck of contributors waiting for CI jobs on the shared Mill repo by moving CI for draft/WIP pull requests to each contributor's own fork, each of which has a separate concurrency budget (~20 concurrent workers) from the com-lihaoyi organization.
Once a PR is marked ready-for-review, com-lihaoyi CI begins, so the author effectively has a choice of where they want to run their CI workloads. Mill's main PR validation CI jobs are portable, do not need any special secrets or environment, and can run just fine in contributor's GH Actions environments
To do this:
-
Made the upstream
build-linux/build-windows/test-docsCI jobs filter ongithub.event.pull_request.draft == false, and -
Removed the
branches: - mainfilter on therun-testsjobs so they begin running in contributor GH Actions when commits are pushed to a branch -
Add a new
Draft CIworkflow that pushes a github status linking to the PR's branch in the contributor's fork, as an easy way to see the CI history for the PR there. This needs to run onpull_request_targetrather than justpull_requestto have the suitable permissions to update statuses -
Some subtleties around handling
ready_for_reviewevents such that when a contributor marks a PR as ready for review, CI is re-triggered and handled appropriately without needing to subsequently push a new commit. -
Substituted
github.head_refwithgithub.ref_namein theconcurrency.groupkey since the former only works on PR events while the latter also works on push events
This is a bit of an unusual setup AFAIK for Github Actions, but I've tested it manually a bunch and it seems to work, and should mitigate the bottleneck of multiple people updating their WIP PRs and having everyone blocked waiting on everyone else. Worst come to worst, if you mark a PR as ready_for_review it falls back to the old behavior, but this setup gives you an option to run draft PR validation on your own fork's GH Actions where it won't get blocked by other people's PRs. This is something you can actually already do today, but it requires some tedious fiddling that PR makes seamless
Tested on https://github.com/com-lihaoyi/mill/pull/3540 via git commit -am . --allow-empty && git commit -am . --allow-empty && git push origin head && git push upstream head~1:test-non-draft && git commit -am . --allow-empty && git push origin head, and targeting this PR against the test-non-draft branch in the Mill repo
So IIUC, PR opened against Mill repository automatically run on Mill CI or the PR creator CI account, depending on the draft status of the PR? Do PRs need to be branches in the users fork or in the Mill repo or are both options supported? Also, if PRs are branches in the users forked repo, don't they already run in their CI?
Currently all PRs run in the com-lihaoyi/mill github actions. We previously disabled all branch-push triggers except for main, and PRs are typically from other named branches in the forked repo. PR validation runs on pull_request triggers, which run in Mill's own environment. This change removes the branch-push filter and filters the pull_request trigger to only apply to non-draft PRs
The current proposed change should work the same for both external PRs as wel as PRs from com-lihaoyi/mill's own branches; in both cases, the choice of trigger depends on whether the PR is a draft or not. Except for internal PRs they both end up running in the same place
So, this change is all about not running draft PR events but run all push events in any branch instead.
It looks to me, that we now run all commits to non-draft PRs, pushed to com-lihaoyi/mill, twice? One push event and one pull request event.
I started to open PRs from the main repository, since this allows scenarios like opening PRs against another PR branch, which gets autoamtically rebased to main, once the dependency got merged to main. To avoid duplicate runs, we now should always open PRs from forks, which is not the end of the world, but slightly less convenient.
Yes I believe you are correct that as of now this change means non-draft PRs from the main repository will run CI twice. Let me see if I can tweak the github actions conditionals to mitigate that