Cabal: Can we stop using the default GitHub merge messages?

[ezyang]

The default merge message looks something like this:

commit 0d6c16582d3a3efe1ba1a8cd0d8b51294aec8a1e (origin/master, origin/HEAD)
Merge: 5190be6a5 a124162ef
Author: Mikhail Glushenkov <[email protected]>
Date:   Sat Mar 31 22:53:32 2018 +0100

    Merge pull request #5244 from grayjay/solver-debug-flags-job

    Use GHC 8.2.2 and new-build for solver-debug-flags Travis job

This means that in one-line commit mode, all I get is:

Merge pull request #5244 from grayjay/solver-debug-flags-job

This is supremely unhelpful when I am trying to find patches that are likely to have caused a regression, e.g.
as in https://ghc.haskell.org/trac/ghc/ticket/14972

My recommendation is that we go back to squashing on merge, rather than attempting to preserve the exact history of a patchset. Short term memory versus long term memory.

[grayjay]

It would be nice if we could use the pull request title and description as the commit message for the merge commit. I don't know whether there's a way to change the default merge message on Github, though. I would rather not lose the individual commits by squashing, since i think they are useful for understanding history, and I like the finer granularity when bisecting.

[hvr]

My recommendation is that we go back to squashing on merge

I'm totally ok w/ squashing unstructured PRs (in fact, I'd prefer that), but if I spent a lot of time making sure that each commit was logically structured (git rebase -i ftw) and also each commit builds (essential for git bisect), I'd strongly refuse to squash it (otoh, there's still the "rebase" variant) :-)

[angerman]

👍 for rebase from me... if that can be done trivially with the github UI.

[ezyang]

if I spent a lot of time making sure that each commit was logically structured (git rebase -i ftw) and also each commit builds (essential for git bisect), I'd strongly refuse to squash it (otoh, there's still the "rebase" variant) :-)

Surely one can also budget the time to open N PRs per commit?

[hvr]

Surely one can also budget the time to open N PRs per commit?

Sure, as soon as GitHub finally adds support for declaring inter-dependencies between PRs (one of the things that Phabricator supports that GitHub doesn't...) ;-)

[23Skidoo]

I prefer rebasing as well, for the same reasons as @hvr.

[ocharles]

@hvr What does Phab do that GitHub doesn't here? You can open a PR against a different branch, but I guess Phab does more than this (I wouldn't know, I haven't used it).

[chreekat]

Alas, I was nerd-sniped by this issue, and created git-pull-merge. It dog-fooded itself, creating this merge commit from this pull request.

I also can't stand the default commit messages, so I often do merges from the comfort of my own shell. Tonight's insight was that I can scrape the PR info using an API call that needs no auth (for public repos, anyway). That, plus some formatting with jq, is enough to create a decent merge commit.

edit: Renamed to git-accept

[BardurArantsson]

As long as the PR commits are neatly separated and aren't just junk like "wip", "fix previous commit", etc. then rebase-merge is hugely preferable. Otherwise, yes, please squash-merge.

(I say we can probably leave it up to the discretion of the person doing the merging and/or the reviewer asking for the merge?)

[23Skidoo]

How about changing the default merge PR action to "rebase"?

[23Skidoo]

I changed the settings to make rebase merging the default.

[phadej]

Based on recent experience, I think rebase and merge is bad:

• It rebases unnecessarily (rewrites history, this is bad on non-technical reasons)
• we lose connection to particular PR completely. They contain a lot of helpful discussion.

So I think we went from bad (= bad one line commit message) to worse.

[ezyang]

we lose connection to particular PR completely. They contain a lot of helpful discussion.

I just realized that "rebase and merge" doesn't modify the commit message to include the PR number, while "squash and merge" does. So I agree, rebase and merge is worse than merge, or squash and merge. My preference is for squash and merge but I agree with @phadej that rebase and merge is definitely bad and we shouldn't do it.

[chshersh]

I'm voting for Squash and Merge. This option is useful, because:

• Keeps link to PR
• Doesn't spoil commit history with intermediate commits (like Update changelog, Fix typo, Fix Travis, Fix tests) and doesn't force users to make squashes manually at the same time
• Allows to edit resulting commit message so it can contain useful information at the end
• Keeps history linear and make it easier to rebase over master branch

In all my projects I use Squash and Merge by default. Didn't notice any drawbacks after using it for a year in projects with multiple contributors.

[23Skidoo]

I've re-enabled squash merging and merge commits, and disabled rebase merging. So now whoever pushes the merge button can make the decision between squashing and normal merging.

[hvr]

btw, another thing that's convenient with (non-squashed) merge commits over rebased merges: git cherry-pick -x -m1 <commit-hash> works and generates a squashed cherry-pick collecting the commits contained in a non-squashed merge; however, you have to remember to use Co-authored-by: ... or verify the Author-header is accurate, to retain accurate authorship if you merge someone else's work.