CI: Migrate from Bors to GitHub Merge Queue #110
Conversation
| # We don't actually want to run integration tests on pull requests, | ||
| # because we want to avoid hitting rate limits. | ||
| # So, if this is a PR build, mark the integration tests as "skipped". | ||
| if: github.event_name != 'pull_request' |
There was a problem hiding this comment.
Do I understand correctly this is a required job and so skipping it is making all PRs unmergeable? That's at least my explanation for this PR waiting for a job which will never run: https://github.com/JuliaLang/BumpStdlibs.jl/queue/master
There was a problem hiding this comment.
Do I understand correctly this is a required job and so skipping it is making all PRs unmergeable? That's at least my explanation for this PR waiting for a job which will never run: https://github.com/JuliaLang/BumpStdlibs.jl/queue/master
GitHub has a very weird rule: if a required job is marked as "skipped", then GitHub considers it as having satisfied the requirement.
E.g. on this PR, GitHub says "all checks have passed".
There was a problem hiding this comment.
Yeah, because you aren't running much. But this job is necessary for merging, in fact this PR was removed from the merge queue in the meantime. I see two options:
- don't make this job mandatory
- don't skip it for PRs
There was a problem hiding this comment.
But this job is necessary for merging, in fact this PR was removed from the merge queue in the meantime. I see two options:
So, the overall workflow (CI) needs to start before GitHub marks this job as skipped. That can't happen until this PR takes the lock. So what happened was just that it took a long time to take the lock, and the merge queue timeout (the separate timeout that lives in the repo settings) was exceeded.
I've increased that timeout.
If we set the merge queue timeout to be really high (which I've just done), and wait, then eventually the PR will be merged, it's just a matter of waiting a very long time.
To clarify again: this PR can absolutely be merged with a status of "skipped" for the integration tests.
There was a problem hiding this comment.
In other words, with GitHub Merge Queue, it is very much allowed to both (1) Make a job required, and (1) mark it as "skipped" on PRs. We currently do this exact thing on RegistryCI.jl and CompatHelper.jl.
But yes, because of the global lock, you often have to wait a very long time to get the lock, so I usually set the merge queue timeout to the max allowed timeout (6 hours).
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
|
I'll demonstrate this now. Currently, if you look, the "integration tests" status says "skipped". I'll click the "merge when ready" button now. |
|
I don't see any changes with before, the |
|
The integration tests are currently running right now on the |
|
The merge queue view often doesn't tell the whole story. You usually have to go to the Actions tab. |
They did also 6 hours ago, and yet the PR wasn't merged: https://github.com/JuliaLang/BumpStdlibs.jl/actions/runs/12612323356 |
Because (at that time) the merge queue timeout was only set to 2 hours, and the timeout expired before the integration tests finished. |
https://github.com/JuliaLang/BumpStdlibs.jl/actions/runs/12612323356/job/35148946271 finished running at 17:31 UTC, but the PR was removed from the merge queue at 20:21 UTC, it doesn't look like a timeout issue to me. |
Aha. Okay, now I know what the bug is. It's a different bug. It's the job name bug. I'll fix that now. |
|
Basically, for my approach to work, the job name can't have anything like e.g. the Julia version number in the job name. |
|
Alright, I've fixed the job name, and I've fixed the other bug (we can't use a matrix for the integration tests). |
|
Alright, so now I have done:
We should now actually be good to go. Let's try it out. |
|
Now I can see all expected jobs running: |
Yeah, we needed the changes in 5f43de2 |
The free public instance of Bors has been shut down.