Switch merge strategy of Github pull requests to "squash and merge"

[callmetango]

Currently we use the Github standard settings for merging pull requests like so:

The default is "Merge pull request" which merges all commits of the pull request into the target branch. One by one. This may pollute the Git history and make code changes non "atomical".

To have a tidy commit history the contributor is in charge to squash his commits into one and force push these onto his feature or bugfix branch. This is anything but a good workflow since:

• it puts the burden on the contributor to squash his commits
• the one merging the pull request might miss the fact that there are several commits to the branch to merge
• team work on a pull request is hindered since sqashing commits is rewriting the history which is a Bad Thing
• the code review feature in Github may loose track of what has been commented on etc. since the history gets rewritten by squasing commits

I suggest the following:

• We should use the "Allow squash merging" option as the only option allowed to merge pull requests.
• We should set the "Default commit message" to "Pull request title and description"
• We should make this configuration change to all our repositories

That way we get a nice clean commit history and "atomical" changes in only one commit which is easy to track down.

Additionally, we should allow for "Always suggest updating pull request branches" so new changes to the base branch can be easily updated via the Github web UI.

For detailed information on this concrete topic: Git Rebase vs Git Merge: Never Force Push Again | Understandable

Some more information on Git rebase being bad: Git rebase, squash...oh my! - mgasch.com

@artist4artix @dec05eba @metux @mikedld @stefan11111 What do you think?

[dec05eba]

Because of how the way github works with reviews and fixes I agree that squashing is better, it also allows git bisect to work better

[stefan11111]

I don't think squashing all the commits from a pr is a good idea.
Instead of 10 small commits, you would get one large commit with all the changes.

[callmetango]

I think it depends of the type of changes. If this is a smaller feature or bugfix then the commits may be squashed without losing any information. For bigger changes it might be good to keep separate commits, that's right.

[CustardEddy]

(Verbose answer, as others reading may not fully understand this subject...)

I think I get where you're coming from, but if I'm right I think these are orthogonal issues:

1. Should big commits be done at all.
2. How to handle commit history.

Generally, I agree, many smaller commits are probably better, especially if refactoring to support future work. Presumably, such refactorings would stand on their own and thus provide immediate benefit; these should be integrated early to help weed-out regressions sooner.

For feature commits having a single commit that shows, in one slice, all the parts of the code that were changed, can be pretty useful for getting a feel of what's going on.

What value does the commit history of a working branch add? For release-on-release changes, no one cares: one only cares about what changed between version A and version B, not 'how you got there'. Especially if this is 6 months or more after the merge. In a very real sense, intermediate commits on a working branch is "code that never was"; people tend to work with "code that is."

If there is some context that would be 'revealed' by the commit history, then, surely, this should be documented, either separately and/or directly within the relevant source files? (Even if the code comment is "see docs/XYZ" for rationale.

I think this last point is especially important when using Git, as, explicitly, history is malleable. (Bring back SVN! 😉 😆 )

Based on my experience, I'd favour the rebase + merge/sqush-history approach in almost all circumstances. (There's very little value IMNSHO of a 'merge from master' commit...)

[stefan11111]

(Verbose answer, as others reading may not fully understand this subject...)

I think I get where you're coming from, but if I'm right I think these are orthogonal issues:

1. Should big commits be done at all.

2. How to handle commit history.

Generally, I agree, many smaller commits are probably better, especially if refactoring to support future work. Presumably, such refactorings would stand on their own and thus provide immediate benefit; these should be integrated early to help weed-out regressions sooner.

For feature commits having a single commit that shows, in one slice, all the parts of the code that were changed, can be pretty useful for getting a feel of what's going on.

What value does the commit history of a working branch add? For release-on-release changes, no one cares: one only cares about what changed between version A and version B, not 'how you got there'. Especially if this is 6 months or more after the merge. In a very real sense, intermediate commits on a working branch is "code that never was"; people tend to work with "code that is."

If there is some context that would be 'revealed' by the commit history, then, surely, this should be documented, either separately and/or directly within the relevant source files? (Even if the code comment is "see docs/XYZ" for rationale.

I think this last point is especially important when using Git, as, explicitly, history is malleable. (Bring back SVN! 😉 😆 )

If I'm looking for a regression, having smaller commits means that it is easier to find what actually went wrong.
I'd rather know 'somewhere in this 100-line commit something went wrong' than know 'somewhere in this 10k line squashed commit something went wrong'.

Based on my experience, I'd favour the rebase + merge/sqush-history approach in almost all circumstances. (There's very little value IMNSHO of a 'merge from master' commit...)

Indeed, we shouldn't have 'merge from master' commits, but those can be avoided with a rebase.

[callmetango]

I like the discussion and different points of view. Regarding what have been said already, I get the feeling that we should have a discussion on Git merge vs. rebase and how to handle branches, be it for feature development, bug fixes or long lived releases. So the whole picture of our branching and merging strategy. After this we could clarify for this "detail".

[callmetango]

Closing this idea in favor of a broader Git branching and release strategy.

[callmetango]

See Create a Git branching and overall release strategy · Issue #281 · X11Libre/misc