Repository navigation
github commit queue feature #1110
Description
can we get on the signup for this: https://github.com/features/merge-queue/signup
can we get on the signup for this: https://github.com/features/merge-queue/signup
Activity
mcollina commentedon Oct 28, 2021
cc @nodejs/tsc
targos commentedon Oct 28, 2021
You mean for
nodejs/node? I think we would still not be able to use it for the same reasons we can't use the merge button.joyeecheung commentedon Oct 29, 2021
@targos hmm, wait, remind me again why we don't use the merge button? (provided that one updates the commit message properly?)
targos commentedon Oct 29, 2021
joyeecheung commentedon Oct 29, 2021
oh, right, the merge queue feature of GitHub currently only makes sure that stuff are checked. We need some automation to edit the commit messages also - unless we ask people to edit their commits in the PR branch prior to landing..
Trott commentedon Oct 29, 2021
Even then, we'd need to add metadata at landing time.
Don't tell anyone, but I've successfully used the squash and merge button once or twice by adding all the metadata with node-core-utils, then force-pushing to the PR branch. From there, the squash and merge button is usable for single-commit PRs. (It adds the GitHub PR number to the commit message title, but that can be manually removed and isn't a big deal if you don't remove it.) Doing that is more work than following the usual procedure, though. So, not a big win.
ljharb commentedon Oct 29, 2021
None of the documentation for merge queue seems to describe how the change is merged either - a PR could have 1-3 mechanisms available to it, and could always also be ff-merged from the command line. Is this configurable?
aduh95 commentedon Oct 29, 2021
Kind of off topic, but I wonder if we could use GitHub CLI tool to do that from the commit-queue for one-commit PRs: https://github.com/proxy/cli.github.com/manual/gh_pr_merge
mmarchini commentedon Oct 29, 2021
This seems like a good feature for repositories other than nodejs/node though
Trott commentedon Oct 29, 2021
Step one would be to find a way to get the commit-queue to force-push to the PR branch. That will break for certain PRs. (The npm bot PRs, for example, don't allow us to force push to the branch.) But it can fall back to the current push-to-master-and-close-the-PR process that it does currently. For everyone else, the force-push will give us the nice purple merged PRs and improve some of our ability to gather metrics.
And once you have that working, then there's probably no reason not to do it if you have the commit-queue landing multi-commit PRs. And you can use the GitHub CLI tool to merge with the
--rebaseoption.aduh95 commentedon Oct 29, 2021
If we were using the
--squashoption from GitHub CLI tool, we wouldn't need to force push I think.I'll look into that once nodejs/node#40577 has landed (if it lands).
EDIT: I just tried that in nodejs/node-auto-test#34, it unfortunately adds an unwanted first line that consists of
PR title (PR number). Too bad that cannot be disabled, otherwise it would be quite useful for our CQ.ljharb commentedon Oct 29, 2021
You'd still want to force push tho any time you could, no?
aduh95 commentedon Oct 29, 2021
No, I'd prefer not to force push, it makes the GitHub Actions CI run twice.
ljharb commentedon Oct 29, 2021
That's a benefit - it's supposed to run on the final commit that will land on master, before it lands on master. This also gives you the opportunity to freshly rebase it before the force push - otherwise anything that lands on master between the last CI run and the merge could cause master to become broken.
Either way, CI on GHA on a public repo is free, what's wrong with running it 20 times, let alone twice?
aduh95 commentedon Oct 29, 2021
It's free, but limited, it's not uncommon to have macOS jobs queue for hours when there are a lot of PRs (also, global warming, why run useless CIs?).
ljharb commentedon Oct 29, 2021
If that was a concern for Github, they'd provide the ability to only rerun failed jobs instead of only rerunning every job :-) but fair enough.
nschonni commentedon Oct 29, 2021
May be worth looking at https://github.com/marketplace/actions/skip-duplicate-actions if you want to trim overlapping runs
targos commentedon Oct 31, 2021
FWIW I signed up the org for the feature. We can still decide not to use it.
mex511511mno commentedon Nov 18, 2021
mhdawson commentedon Apr 5, 2023
There has been no discussion/updates on this for over a year and a half. I'm going to close please re-open (or let me know if you can't do that yourself) if that was not the right thing to do.