[SciPy-Dev] On the topic of separate merge commits

[Eric Larson]

I'm okay with each maintainer following their preference, as there are
arguments either way.

That said, if people don't care much either way or want to think about it,
I'd advocate for "squash and merge" as the default policy. I started using
it (whenever they first introduced it years ago) across multiple repos with
no observed practical drawbacks so far. On the other hand, it has improved
maintainability in at least the following ways:

1) It makes `git bisect` easier as we are (almost) guaranteed that tests
worked before the commit and after the commit. It's not 100% guaranteed
because maintainers might miss a red CI/failing test in a PR, but this is
rare. When we merge without squashing, intermediate commits often have
errors. Squash+merge means there is no need for skipping steps in the
bisect, all can be good/bad.
2) It keeps history smaller.
3) It makes a nicer/more direct mapping between commits to scipy:master and
PRs.
4) Because of 1-3, browsing commits and running `git bisect` are faster.

The only real disadvantage as I see it is that individual commits are lost
(the flipside of (2)), so if these are really helpful being split for some
reason, you'd have to go back to the original PR / branch to see them
(assuming the branch is not deleted). For what it's worth, though, I have
not run into a case where split commits are actually helpful across many
`git bisect`s and many PRs across repos, so to me it's rare enough not to
outweigh the benefits.

The few remaining times I do a plain `merge` is when I see that the commits
in the PR all have green CIs (or at least were likely to if they had run)
and probably could have been individual PRs on their own (e.g., "move
function", "fix bug", "add enhancement" all as part of making one thing
more usable in a single PR). Some contributors are very good about
structuring/restructuring their PRs this way, but it seems to be the (rare)
exception and not the rule.

If nobody objects to this line of reasoning, it might be worth putting a
"if you don't care, make your default squash + merge and use it; if you do
care, use your best judgment" in the maintainers guide somewhere. If people
don't buy my arguments, then we should just put "use your best judgment".

My 2c,
Eric


On Wed, Aug 25, 2021 at 7:15 AM Evgeni Burovski <evgeny.burovskiy at gmail.com>
wrote:

> > On Tue, Aug 24, 2021 at 4:51 PM Tyler Reddy <tyler.je.reddy at gmail.com>
> wrote:
> > >
> > > > Hence I thought you would be the best placed to answer my
> interrogations.
> > >
> > > I am certainly not the appropriate person to target for
> "interrogations" on this matter. It is a long-standing practice in the
> community, and a feature GitHub has in their GUI to create separate merge
> commits even when there is only a single commit in the PR.
> > >
> > > If SciPy or the community strongly prefers not to do it, then I'll
> switch to squashing, but until then I strongly disagree that I should have
> to defend PRs where I do that.
> >
> > I must say - that's what I tend to do (merge, not squash).   I can see
> > arguments for squashing, if the history is very messy, but if the
> > history is good, and the commits are well organized, squashing seems
> > like the wrong thing to do.   Why not leave it to the judgement of the
> > person doing the merge?
>
> I'd second what Mattew said.
> (I would also create a merge commit without much thinking, btw).
> There are clear limiting cases (messy history; a single 5kLOC commit
> etc), but otherwise it's really a judgement call of a person who does
> the merge.
> While Github does offer a bunch of options, deciding to just not think
> about the merge mode is a perfectly valid strategy. I personally don't
> think we need to standardize it, or even think about it too much.
> In the project of our size, there's going to be some variation between
> maintainers, and it's likely OK. One person prefers to squash, the
> other prefers a merge commit --- and it's fine IMO.
>
> Cheers,
>
> Evgeni
> _______________________________________________
> SciPy-Dev mailing list
> SciPy-Dev at python.org
> https://mail.python.org/mailman/listinfo/scipy-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.python.org/pipermail/scipy-dev/attachments/20210825/6feedfc0/attachment-0001.html>