Enforce linear history of main branch with git rebase or squash?

[andthum]

Should we enforce a linear history of the main branch?

Currently, pull requests to the main branch require before merging

• At least one review
• Passing status checks
• Resolved conversations
• Signed commits
  (See About protected branches)

I wonder if we also should enforce a linear history be either rebasing or squashing?

[ezavod]

I think it's a matter of taste. What do you prefer?

Personally I find a linear history easier to understand and prefer rebasing the PRs. However, to keep things manageable, I would recommend that a contributor should squash their own commits to logical chunks in a PR before merging.
As an example the five commits
C1: "Introduce feature A"
C2: "Fix something in feature A"
C3: "Introduce feature B"
C4: "Update feature A"
C5: "Revert parts of feature A"
can be squashed down to just two commits:
C1: "Introduce feature A"
C2: "Introduce feature B"

Pro: Clean linear history that is easy to understand and bisect.
Cons: Puts burden on the committer of the PR. They have to perform the rebase and squash which might be difficult and error prone for new contributors.

[andthum]

My understanding of "squash and merge" is that after a PR is merged there is only one single commit containing all the changes.

That's also my understanding.

IMHO this makes reverting changes much more difficult than a non-linear history would.

I totally agree with you. So, we probably should disable squash merges at all. As you mentioned, the only use case for squash merges that I see are PRs with a manageable number of commits but many back and forth changes (like #10 and even more #16). Optimally, contributors should have cleaned their commits to not contain back and forth changes before creating a PR, although we shoudn't and cannot enforce contributors to do this, but we should encourage them to do so.

This leaves "rebase and merge" as the only other viable option. Assuming one would like to have a linear history due to simplicity this would be the only option, right?

Right. So, do you agree that our normal workflow should be to merge PRs with the main branch via a rebase merge and afterwards delete the head branch? Then, the only exception from this workflow would be, when further work is planned on the head branch. In this case we would have to do a normal merge and of course must not delete the head branch after merging.

[ezavod]

Yes, agree 👍
I think it would even be possible to merge a feature branch (I read your 'head branch' as feature branch == stuff in PR != main) with rebase merge instead of a normal merge even if future additions are planned. But these exceptions can be discussed when they come up.

[andthum]

When I tried to rebase PR #16 onto main, I encountered the following problem:

"Base branch requires signed commits. Rebase merges cannot be automatically signed by GitHub."

The reason for this is that I enabled the branch protection rule "Require signed commits" for the main branch, but when rebasing a PR with GitHub, GitHub always creates brand new commits, which are therefore not signed. From the Docs:

The rebase and merge behavior on GitHub deviates slightly from git rebase. Rebase and merge on GitHub will always update the committer information and create new commit SHAs, whereas git rebase outside of GitHub does not change the committer information when the rebase happens on top of an ancestor commit.

One solution would be to not require signed commits. But by now, I think enforcing a linear history of the main branch does more harm than good. A basic merge will almost always pass smoothly. You will only have to care about potential merge conflicts, but this can happen with all merge methods. On the other hand, squashing and rebasing involve a change of history and are therefore per se more dangerous and error prone. Moreover, squashing or rebasing is not suitable for every branch and you have to be more careful, when using these methods. For example, if you want to continue work on a feature branch that has already been merged to another branch via squashing or rebasing, you have a higher chance to run into merge conflicts during future merges. See e.g. About pull request merges and About merge methods on GitHub for issues that you have to consider when using squashing or rebasing.

And as you said in your first reply, having a linear history is a matter of taste, as also outlined in the Git book. Do we want a "record of what actually happened" or a nice "story of how your project was made"?

So, long story made short: Here is what I think by now about the different merge methods.

Basic merge (non-fast-forward GitHub merge that always creates a merge commit):

• Pros:
  • Extremely easy to use.
  • When looking at the history, feature work is always (due to non-fast-forward merges) encapsulated in feature branches instead of being incorporated in the main branch. I think this has also advantages when we have to roll back some work, because you can easily see, where the work on a feature begins and where it ends.
• Cons:
  • Non-linear history, which can become messy and than it might be difficult to roll back changes.

Squash and rebase merge:

• Pros:
  • Result in clean, linear history.
• Cons:
  • You have to take more care, because you are rewriting the history of already published commits.
• Cons particularly for squash merges:
  • All commits of the PR are squashed into one commit on the main branch. This makes it impossible to undo only specific commits of the PR.

At the moment, I do not see the great benefit of having a linear history and would rather go for basic merges.

[ezavod]

[...] GitHub always creates brand new commits, which are therefore not signed.

Good to know that GH works in a different way compared to git. On CLI I have never encountered this problem when rebasing. (Tangentially, I wonder how hard it's for new group members to setup signing for their commits.)

At the moment, I do not see the great benefit of having a linear history and would rather go for basic merges.

Also, I see your points. In general my preference is still having "a nice story" instead of "what actually happened" when considering the mess that I typically produce :-) But, all things considered I understand your argument and agree that a basic merge is the best way forward. So choose the method that makes the maintenance the easiest for you.

However, I don't fully get the hint at merge conflicts. They have to be solved irrespectively of merge method. I believe that this is ideally done by the author of the commit. This means that a rebase might be necessary anyway. There might exist some fancy button on Github that does some magic, so we can look at this scenario when it comes up.

Edit: When creating a merge commit (i.e. basic merge) the conflicts have to be handled by the person who actually does the merge, haven't they?

[andthum]

On CLI I have never encountered this problem when rebasing.

I guess I should also switch from the GitHub web interface to the CLI.

I wonder how hard it's for new group members to setup signing for their commits.

That's a point, but it is nicely explained in the GitHub Docs. I'm also new to Git and GitHub and I also managed to sign my commits. It's of course not such important for the MDTools project, but without signed commits/tags you cannot be sure who is the author of these commits/tags.

I don't fully get the hint at merge conflicts. They have to be solved irrespectively of merge method.

If you use squash or rebase to merge a feature branch that was already merged before, you are again squashing or rebasing all commits since the feature branch has been created (at least if you use GitHub's web interface), including all the commits that have already been merged. If the main branch has moved on in the meanwhile and someone else alters some changes that you have introduced with your first merge, you will run into merge conflicts when you merge your feature branch a second time. This applies at least to squash merges according to the GitHub Docs:

"If you continue to work on the head branch, then create a new pull request between the two branches, the pull request will include all of the commits since the common ancestor, including commits that you squashed and merged in the previous pull request."

When creating a merge commit (i.e. basic merge) the conflicts have to be handled by the person who actually does the merge, haven't they?

I think the GitHub web interface does not allow you to merge a PR if there are any merge conflicts. The admin can than ask the author of the PR to resolve the conflicts.

I think we have to postpone this discussion until more specific use cases have come up.

[andthum]

As I got now more familiar with Git, I think we should establish this Git-workflow for MDTools. It is basically the same workflow that we agreed on for rs@md.

Regarding the rebasing and squashing discussion:

When merging a PR, we will use the simple merge that creates a merge commit. However, we will enfore contributors to rebase their topic branch onto the target branch before merging. In this way we will have a quasi-linear history. And we will encourage contributors to squash, split and reorder their commits in logical chunks.

I think we can close this discussion now.