Squash merging for a linear history

[SimonAlling]

I was told in #16796 that I should rebase my PR branch and squash its commits (so that it would contain only one commit) before it could be merged. I didn't want to do either, so I tried to find out what that was all about.

The contribution guidelines indicate that merge commits are generally undesirable:

Please use fetch / rebase (as shown above) instead of git pull. git pull does a merge, which leaves merge commits. These make the commit history messy and violate the principle that commits ought to be individually understandable and useful (see below). You can also consider changing your .git/config file via git config branch.autoSetupRebase always to change the behavior of git pull.

💡 Note that the paragraph above doesn't concern existing PRs; it's about what contributors should do before creating a PR.

I fully agree that merge commits make the history really hard to read and that PRs should indeed not be merged as merge commits. But I don't think rewriting the history of existing PRs is the best way to achieve that goal, for the reasons outlined below.

• While merge commits cause an absolute mess in the trunk history, they are not a problem in a PR context – quite the contrary.

• Some ~30% of all commits on main (even recently) are merge commits, so the no-merge-commit policy currently isn't effective even on the trunk.

• Consider this typical scenario: A PR consisting of a single commit is created. Later, someone realizes that something must be fixed before merging it (due to review feedback, CI failure, merge conflict, new changes on the trunk, etc …).

  The best-practice and most sensible thing to do in that situation is to push another commit, mainly for these reasons:

  • Rewriting public history without good reason¹ goes completely against the fundamental principles of Git.
  • If existing PRs frequently have their histories rewritten, multiple people cannot collaborate on a PR.
  • Squashing and rebasing makes it much harder for reviewers to understand how a PR has changed since they last reviewed it, compared to pushing the fix as another commit.

  This is in fact implicitly supported in the contribution guidelines:

  Commit changes made in response to review comments to the same branch on your fork.

• GitHub has native support for squash-merging: just select the Squash and merge option when merging a PR², and it ends up as a single commit on the trunk, which can thereby be kept perfectly linear. This can even be enforced by disabling Allow merge commits and Allow rebase merging (keeping only Allow squash merging) in the repository settings. (There is also a Require linear history option for extra protection.)

Therefore, I suggest that the "one commit per PR" requirement be abandoned in favor of a squash-merge workflow with only squash-merging allowed, and that contributors be allowed to push additional commits to existing PRs. This will improve the quality of the trunk history, as well as the PR experience for contributors and reviewers alike.

Footnotes

1. Signing off commits retroactively, for example, requires that history be rewritten, of course. ↩

2. GitHub unfortunately generates really bad commit message suggestions when this feature is used on a PR with multiple commits (or at least that used to be the case), so care needs to be taken to make sure a nice and clean commit ends up on the trunk. ↩

[AllForNothing]

@OrlinVasilev Can you help to have a look at this?

[SimonAlling]

Any updates, @AllForNothing or @OrlinVasilev?

[Vad1mo]

I am also a big fan of not rewriting the history. Not only you lose the history and track of everything. Epically if you want to track if a contributor did the changes you asked them to do aren't tracked properly. You have to always jum into code.

The benefit of having one commit on the trunk can be accomplished without rebasing code and squashing.

As I and I guess we don't want to make our life and the one of contributors to complicated we should just use squash-merging if there is more then one commit.

Here is a nice visual summary -> https://cloudfour.com/thinks/squashing-your-pull-requests/

Explicit merges

Implicit merge via rebase or fast-forward merge

Squash on merge, generally without explicit merge