Squash commits when merging

[Marcono1234]

Most or all pull requests against this repository are currently merged with a merge commit. This makes the Git history of main rather verbose and possibly difficult to navigate:

• Some commits are just a one line typo fix
• Some commits fix issues introduced by the same pull request (which therefore never really existed in production main)

Have you considered squashing commits when merging pull requests? Or is there a reason why you refrain from doing this?
This would probably lead to a cleaner Git history on the main branch.

Note that I am not blaming the pull request authors for making typos or other mistakes, I am just questioning whether there is any value in having separate commits for them on the main branch.
Similarly I am not suggesting to squash commits of all pull requests. There are situations, especially for large pull requests, where it is desired to keep the separate commits on the main branch.

[Marcono1234]

Would still be interested in your opinion on this because I think all of this still applies. One additional issue is that usually within a PR the commit messages are specific to that PR, e.g. "Add change note", which is perfectly fine, but in the history of main it provides not much information; though using more verbose messages for all PR commits would not help much because then the main history would be even more verbose. It would here as well be cleaner if the commits were squashed.

Another disadvantage might be that the repository size increases faster when not squashing the commits.

[jbj]

We've had some internal debate about this, coming out vaguely in support of the status quo. I think it's ultimately a matter of personal preference, and there's a long list of pros and cons that's well known and not specific to this repository.

With our current conventions for merging, the best place to view logical changes is the list of GitHub PRs. When navigating the commit graph, following the left parent pointers in main will keep you on a straight line of merged PRs.

[jsoref]

As someone who makes many commits (And many PRs), I find the way projects tend to use squash commits to be pretty lousy (my PRs often have dozens or hundreds of commits with similar messages plus DCOs to make bots happy, which means the default squashed message is just a lot of DCOs and blank lines). I personally prefer when projects either manually squash, or do a rebase. In either case, I prefer when projects work with PR authors to ensure that either the commit message or the commits themselves are clean.

I understand the frustration of looking at merge commits https://github.com/github/codeql/network doesn't seem like a fun page to load:

Unless a project is really afraid of scaring people away, I think it's pretty reasonable to ask them (in their PR) to rebase their own work so that it doesn't include merges of its own -- they're really unhelpful for anyone trying to interpret a story. And it shouldn't cause them significant harm or grief -- if it does, their changes are probably suspect. Similarly, I think it's reasonable to ask contributors to clean up their commit histories to not have "tried this, it's half broken" commits.

The benefit of someone being able to say "I started my work 10 years ago" should be pretty rare. And I don't think this repository does significant backports to the point that it's useful to say "I started this work here, and we can therefore safely merge it into any branch that's in the future" -- that doesn't actually help with backports since most users will start their work at "now" and you'd still need to cherry-pick. (There is a model where people know the oldest target releases that's still around and do all their work from that base which enables them to merge to main, and then merge to older branches because they're all in the future of the starting point, but I don't think many projects actually work this way.)

But, at the end of the day, whatever policy a project picks is really the decision of the project.

[Marcono1234]

I find the way projects tend to use squash commits to be pretty lousy [...] which means the default squashed message is just a lot of DCOs and blank lines

You can edit the commit message when squashing in GitHub, but yes it is easy to forget doing that.

---

I think it's pretty reasonable to ask them (in their PR) to rebase their own work so that it doesn't include merges of its own
I think it's reasonable to ask contributors to clean up their commit histories to not have "tried this, it's half broken" commits

The problem is that rebasing might be complicated for users new to Git, and generally rebasing can be error-prone and if you mess up (and notice it too late), restoring the original changes is also a bit cumbersome. And if you then force push the rebased changes it might also cause issues with existing review comments on GitHub, and you don't easily see the differences between the commits, you can just compare the changes before and after the force push.

Therefore I think the least error-prone way is really through GitHub's squash feature.
Though as you mentioned, ideally contributors would also try to not push dozens of incomplete commits, with possibly meaningless messages (at least for projects which don't squash on merge).

---

But, at the end of the day, whatever policy a project picks is really the decision of the project.

True