[neomutt-devel] GitHub Merging and Squashing
Pietro Cerutti
gahr at FreeBSD.org
Tue Feb 7 21:21:50 CET 2017
> On 7 Feb 2017, at 15:01, Guyzmo <guyzmo+mutt+neomutt at m0g.net> wrote:
>
>
> Before discussing the merge and squash strategies on github, I think a
> small clarification on how to and what for build your commit history is
> needed.
>
> We often give the rule of "make your commits atomic" as a rationale when
> writing commits. But it's easy to make the commits more atomic than
> necessary. So the rationale to keep is to think of how the commit
> history will be used once you're done working on it. And here are the
> common use cases:
>
> 1. read the history to keep up with changes ;
> 2. bisect the history to find when a regression has been introduced ;
> 3. help with merging or rebasing
>
>> On Tue, Feb 07, 2017 at 12:42:15PM +0000, Richard Russon wrote:
>> […]
>> When to merge? When to squash?
>
> when working on a PR, you can have two strategies:
>
> ## Commit as you go
>
> the most straightforward and most common one is to commit as you go:
>
> ```
> do {
> make changes;
> compile;
> test;
> commit;
> push;
> } while (no more changes is needed or requested by review team);
> ```
>
> then you end up with a commit log in the PR that might look like:
>
> ```
> adds feature F with behaviour B
> refactors A to make it simpler
> adds documentation and doxygen
> fixes race condition
> fixes typo in doxygen
> adds related feature F' to cope with behaviour B'
> fixes edge case handling
> updates documentation
> ```
>
> So this commit history is featuring a lot of very small commits with
> very little information, so if we introduce a lot of those:
>
> 1. it's making more painful to read the history (lots of noise surrounding the signal),
> 2. it's going to make bisects take more steps and
> 3. it's not of any added value for the merges.
>
> That's when the maintainer will want to follow through the squash+merge
> strategy. But then the downside would be that the information of the two
> related features is squashed into one:
>
> ```
> adds features F and F' to cope with B and B'
> ```
>
> meaning the maintainer would have to do fetch the branch, squash twice
> manually and only then merge, so it would become:
>
> ```
> adds feature F with behaviour B
> adds related feature F' to cope with behaviour B'
> ```
>
> ## Craft your branch's history
>
> The other strategy, has my preference, it's to "craft" history
> as you're working the code.
>
> When working on a feature (or PR) branch, history is not yet fixed. You
> can craft it, rewrite it, as much as you like to give find the right
> spot.
>
> Instead of having your commits show the order in which you wrote your
> code, you squash as you go:
>
> ```
> make changes
> compile
> test
> commit
> do {
> make changes
> compile
> test
> if (changes related to last commit) {
> commit --amend
> else {
> commit (in a temporary commit)
> rebase -i HEAD~<number of commits you want to see>
> fixup in the older commit
> }
> while (no more changes is needed or requested by review team)
> ```
>
> Then the commit history of the feature branch would look like:
>
> ```
> adds feature F with behaviour B
> adds related feature F' to cope with behaviour B'
> ```
>
> which would contain atomic commits, just big enough to not clutter the
> commit history, without making bisect or merges painful.
>
> Then those can be merged as is, no squashes needed by the maintainer.
>
> The downside of working that way is that by rewriting history, you can
> then lose track of the discussion of why a given change happened (at
> reviewing time). But hopefully the github PR feature keeps track of all
> the commits that were discarded and replaced with the new history, along
> with the commits that led to them.
>
> Actually, this is a strategy that's encouraged by reviewing tools like
> gerrit, rietveld or phabricator (as well as with github and gitlab PRs).
> Of course, it needs the contributor to have a bit of confidence in his
> git-fu than with the other one.
>
> Also, when you think about it, this is pretty close to what's being done
> when one is submitting patches by mail to a project, as when being
> reviewed that way, the contributor resends updated patches, not extra
> commits in follow up mails.
>
>> When work is merged into master:
>> If there are 1 or 2 commits, then fast-forward master:
>> `merge --ff`
>>
>> If there are more than 2 commits, then rebase and merge into master:
>> `merge --no-ff`
>
> Ideally, all branches should be rebased by the branch author to always
> stay based on the master branch so the maintainer's work is made easier,
> and the merge could be fast forwarded (i.e. no merge commit).
>
> But practically, there are cases where the master has had many little
> changes across the code base and doing a merge commit would make it
> easier to address all those little changes at once instead of replaying
> them through each commit of the PR.
>
> Cheers,
>
> --
> Guyzmo
Bravo! :)
--
Pietro Cerutti
gahr at gahr.ch
More information about the neomutt-devel
mailing list