Establish PR merge method policy

[EwoutH]

Currently our commit history is (in my opinion) a mess. There are many small commits on there that are just fixes or small iterations after PR reviews, to get test passing or other minor stuff.

This has several disadvantages:

• It's way less readable
• It isn't clear what was part of one PR
• It's more difficult to use git blame or other tools to view a consistent set of changes
• It risks commit history not being linear
• Cherry picking to a maintenance branch is more complicated
• Reverting unintended behaviour is more complicated

Therefor I would like to discuss a consistent merge policy. On a project I support @quaquel maintaining, we're using the policy below, in which we allow two merge methods. It serves us well and it could be a starting point for Mesa.

Merging Pull Requests

Pull requests should be merged in a way that only atomic commits make it in the main branch. This keeps the git commit history clean and tidy, and therefore useful. This means every commit only pertains one fix or feature, and can't be breaked into multiple commits without breaking things. It also means no merge commits are allowed. For more about atomic commits, see one and two.

There are two ways to do this:

• For (reducing to) a single commit: Use the Squash and merge button in the GitHub UI. Make sure the commit message and description are properly formatted and contain all the improtant information from the PR when squashing.
• For (keeping) multiple commits: Merge from the command line. First make sure all commits are atomic and contain proper commit messages. Use git amend, reset and rebase if nessesary. Then do a fast-forward merge, using:

  git fetch origin
  git checkout master
  git merge --ff-only -S origin/BRANCH-TO-MERGE
  git push origin
  

  In which BRANCH-TO-MERGE the branch is of the PR you want to merge, and -S makes sure you sign the commits.

[Corvince]

I disagree on this one. At least for most of it. Sure, small "fix ruff" commits are kind of unnecessary and as a experienced programmer you should just amend your previous commits. But I think we should also make it as easy as possible to contribute. And in practice I haven't been bothered by it. Honestly I never understood the importance of a "clean" history. What workflows do you have where that matters? What I find myself doing a lot with respect to git history is finding a line of code that I don't understand and then go through the history of that line. This works quite fine in vscode and if there is a "fix ruff" commit its just one click through it to the next meaningful change.

Otherwise I couldn't care less of how the history looks like and if its linear or not. As a software developer I also never worked in a project where this was important or cared for. Sure, everyone strives for "atomic commits", but this is a pipe dream in my opinion. Therefore arriving at a level where we can easily do cherry picking single commits for single features is unrealistic for an open source project with lots of inexperienced contributors.

But I do agree that our PR handling is not optimal, but in the opposite direction. Instead of using squash and merge I would prefer if we have more commits. I have seen several PRs where the logic was contained in a single "atomic" commit. But once review comments came in the implementation was changed and then force-pushed to update the single commit. Even after several rounds of reviews there was only a single commit with the final state, similar to how squash and merge would look like.

But by doing this we loose all the history of how we arrived at that point. Coming back to me looking at single line changes. I usually do this when I don't understand a line or think there is an easier way of doing this. Most of the time I find that "easier way" in the commit history and through the history understand that it missed some corner cases. So I am all for as many commits as possible. As a researcher I just hate losing information ;)

And for a similar reason I would even like to go back to do merge commits. For me its much more obvious that way when and which PR got merged. Its also a lot easier to refresh stale branches than to do a rebase.

The top answer of this Stack Overflow question is worth reading
https://stackoverflow.com/questions/20348629/what-are-the-advantages-of-keeping-linear-history-in-git

[EwoutH]

Thanks for your extensive comment. I thought about it a bit in the past days, you rise a few interesting points. I would take two perspectives:

Me as contributer/user: I also care a lot about the implicit documentation commits and their messages provide. Especially if I want to contribute to something but don’t really understand why it was done this way. But for me, the PR is the historical documentation. In there all the review comments, discussions and the whole process sits there. That’s my historical reference for if I need to know why certain decisions where made.

For the main commit history, I care about two things:

• It’s human readable
• I can immediately see what commits on a high level do and to what effort/PR they are part of.

So I go to the main commit history to see what has changed. If I want more details on why, I dive into the PR.

Me as maintainer: The main commit history is the first thing I look at when hell breaks loose. If we need to revert something because of a critical bug, if we want to cherry pick a bug fix to a maintenance branch, and maybe most importantly, if I need to find what introduced a critical bug that happened between two releases. Then I would do bisection.

For bisecting, I really like my commits linear and atomic. Out of a huge bunch of PRs and commits, I need to find what caused the regressions. A single commit on the main branch should work by itself (atomic), not only with 4 patches applied to them later, without some other commits from other PRs in between (non-linear). If not, it can be really cumbersome to find out.

Once I found it, I can look into the PR for more information.

So this is basically how I see and use a commit history, and why I like a linear atomic one.