suggestion: investigate a commit-queue solution

[refack]

Instead of landing directly on master, contributors push to a commit-queue that validates commit message, code-review meta-data, and possibly runs a CI, only then actually lands on master.
Ref: http://dev.chromium.org/developers/testing/commit-queue

[gibfahn]

Interesting idea, and I know other projects do this (e.g. Rust, which has rollup commits).

What would the advantages be? I find the process of landing commits pretty straightforward today. A CI job to verify commit metadata already exists, it just needs reviving. I think other projects have CI runs that take a lot longer, so it probably makes more sense for them, but it also adds complexity to the landing process.

[refack]

Mainly taking out human error out of the straight forward but manual steps.

• reject on fail of validation of commit message structure & metadata
• reject on CI fail
• Automatic close of PR

If the PR has a good message and could be just squashed:

• automate adding metadata
• automate squash

i.e. turn these cases to a one-click merge

[gibfahn]

What do you mean by commit queue then?

Those are reasons to have a CI flow that runs the commit validation, adds metadata, runs CI, squashes, and lands (although it'd only work for cases where you want to squash), but why do you need a queue?

[refack]

A CQ is in essence a different git remote that colabs can push to, and will either automate the land to master or reject.
one-click-merge is a bonus feature: collab tell CQ to pick up a PR, and CQ does everything.
The design doc by Chromium is quite good http://dev.chromium.org/developers/testing/commit-queue/design

[refack]

P.S. queue is a borrowed term from "mercurial patch queues", the main metaphor is a side storage that can accept or reject without changing master beforehand, and allows for temporary storage of manually rebased commits.

[mhdawson]

This was in place for a short period (basically a job which did the test and if tests passed, landed the PR) but it was backed out because at the time the tests were not stable enough and it would take multiple attempts due to flakes even when nothing was wrong with the PR.

I think we still have a number of flakes on the arm fanned builds and to a less extend the fanned windows builds. I know that I've often add the "arm failures seem infra related so CI is ok". I think we'd need those to be more solid before proposing this again.

[refack]

Sounds legit.
Thanks.

[thomasuster]

@refack Has anyone built a generic solution for this? Like a github integration?

[refack]

@refack Has anyone built a generic solution for this? Like a github integration?

I have no experience with GitHub integrations, but there are gerrit (maybe as http://gerrithub.io/) and rietveld

[refack]

With the inauguration of the Automation SIG IMHO it's a good time to revive this concept.

[rvagg]

For reference, prior art is still here I believe: https://ci.nodejs.org/view/All/job/node-merge-commit/

we can drop in the Jenkins scripts here if that's interesting at all

[maclover7]

Been playing around with Jenkins locally on my machine, and I think I have a job working now where if you provide a PR identifier, like nodejs/node#14998 for example, it will land the commit with PR metadata into the master branch of the repo.

[gibfahn]

Been playing around with Jenkins locally on my machine, and I think I have a job working now where if you provide a PR identifier, like nodejs/node#14998 for example, it will land the commit with PR metadata into the master branch of the repo.

Does it squash commits?

[maclover7]

Does it squash commits?

@gibfahn Kinda sorta -- before figuring out if this is something we want to actually use, we'd probably have to take a look at our contributor policy around commits. Since Jenkins is doing the landing, you'd wouldn't be able to manually amend/squash commits, so puts more of an emphasis on making sure the contributor has the PR commits ready to land -- but I think that's a little premature.

[mhdawson]

No current action item, removing from build agenda based on discussion in last meeting.

[maclover7]

Moving this to nodejs/commit-queue#1, since there is a whole separate team for this now

[sam-github]

Closing as stale, but if anyone wants to take this up feel free to reopen.