Ability to ask wpt-pr-bot to merge on all-green

[zcorpan]

It would be nice to be able to say "@wpt-pr-bot please merge" or something, and have the bot merge when all statuses are green. Sometimes reviews happen quickly but one must wait for the status checks to be green, and it's likely to not happen at all or much later.

[zcorpan]

@foolip, @gsnedders, @Hexcles, @jdm, @jgraham, @plehegar, @sideshowbarker, @thejohnjansen, @tobie, @youennf thoughts?

[Hexcles]

Good idea! IIUC "all statuses" in "when all statuses are green" also include the review bits (i.e. the bot shouldn't auto grant review approvals).

Perhaps something like @wpt-pr-bot auto-merge.

One question is whether the bot should use squash-merge or rebase-merge (or to expose this option to users).

[sideshowbarker]

It would be nice to be able to say "@wpt-pr-bot please merge" or something, and have the bot merge when all statuses are green.

yeah that would be handy

One question is whether the bot should use squash-merge or rebase-merge (or to expose this option to users).

I think a rebase-merge is better — for one thing because (for better or worse) wpt conventions/norms/culture don’t require squashing, and for another because unless somebody is manually (re)writing the commit message that goes in for the squashed commits, we end up with an (IMHO) uglier, longer-but-not-really-unified commit message. It’s better in that case to just have each commit message with its corresponding diff (that is, as we get from a rebase-merge).

[jugglinmike]

If this is to be implemented via git (as opposed to an HTTP request to the GitHub.com API), then we could rebase with --autosquash. That would allow contributors to express more advanced merging instructions using git's built-in functionality.

[zcorpan]

I would be OK with only supporting rebase and merge; usually when I want this it's a simple PR with a single commit.

[foolip]

I'd like this feature, but would probably avoid it if I was not sure about what the resulting commit message would be, since I like elaborate commit messages. If we had to pick one default, I think I'd have to say "squash and merge" since one ugly commit is better than a long chain of commits never intended to be merged individually.

Or, it could just refuse to do anything if there's more than 1 commit.

[zcorpan]

Supporting changing the commit message seems nice, actually. Ok how about this

• if a commit message is provided, squash
• else, if more than one commit, squash
• else, rebase

[foolip]

Squashing for just a single commit is fine too, it'll just append (#XYZ) to the first line, which is kind of handy.

[sideshowbarker]

Supporting changing the commit message seems nice, actually. Ok how about this

• if a commit message is provided, squash
• else, if more than one commit, squash
• else, rebase

Sounds good to me

[jgraham]

That strategy seems to mean you always end up with a single commit. That seems very harmful; over squashing can lose information about the reason for various changes that the author has tried hard to preserve.

In general people should be encouraged to write good commit messages in the first place and not rely on rewriting them during squash (and it should be acceptable to not approve the PR until the commit message is good).

The simplest solution is just to require that the person writing the message chose the right action by saying either wpt-pr-bot:rebase-merge or wpt-pr-bot:squash-merge. There are probably cleverer solutions but I would much prefer forcing people to be explicit than having a bad heuristic.

Also note that there is a security issue here; you have to be sure that only the right people are able to issue squash commands (but! there is a path here to having far fewer people with write access to the repo if you also made wpt-pr-bot able to upgrade reviews from users with read access to approval then it would be possible to implement per-directory review and commit permissions by proxying all actions through the bot).