BS

Squashing commits

Oct 2, 2018 · 6 min read

I recently got involved in a project where I contributed a patch to an inflight feature branch. I then raised a Pull Request into the feature branch, so I could annotate the code in the GitHub PR to explain why I had done it a certain way. After a bit of back and forth the PR was merged into the feature branch.

Later, I found that the feature branch I had worked on had been merged into master, but the entire branch was squashed down into one commit.

Mixed emotions. Ultimately it wasn’t my project, and I had just contributed a small patch, but still.

Version Control

At it’s most basic level, version control gives us: What; When; and Who.

When the feature PR was squashed we lost a substantial amount of the “When” and the “Who”, and was only left with the “What”. This seems wrong to me, however the team had made a decision to work like that. I discussed with the Technical Lead, who explained this was how the team wanted to work, and I should respect that. They are right, that should be respected.

However, this is a new type of project, which was then shown to other people, and my contribution, whilst there, was not attributed to me. So, was this all about me, or is there something to this.

Squashing

Squashing commits is something I do all day long on my own feature branches. I narrate my work how I see fit before I raise a PR to the team. If the team want something to be squashed or split etc, there is a discussion where the team agrees. The reviewer, in my team, does not have the authority to override the way the developer has narrated their code, they can only start a discussion.

I’ve contributed to a fair amount of open source projects, and the lead on those projects always push back to the developer to rebase, set a different base branch, make small changes etc. I cannot remember raising a PR, and the project lead then making substantial changes to that PR before merging.

If you have 3 commits, such as the below

commit 1227109df3d746e3dad19b8af8b63ad45d4b9704 (HEAD -> mojave, origin/mojave)
Author: Ben Selby <ben.selby@me.com>
Date:   Sat Sep 29 15:59:21 2018 +0100

    Deal with the swift version and objc inference warnings

commit 3916021eb03d2c57fcfc412d409985a69494758a
Author: Bob Dylan <bob@dylan.com>
Date:   Sat Sep 29 15:46:10 2018 +0100

    FIX: Set the correct xcode version for TravisCI

    If we do not set this, we get a warning from TravisCI to explain it doesn’t know how to build the project

commit 76637e3b5660e39ccd0a1ad202127b153f690d63
Author: Ben Selby <ben.selby@me.com>
Date:   Wed Sep 26 20:46:43 2018 +0100

    The start of it working on Mojave

and you squash the entire branch, you are left with

commit 1f0edae3e66333389000911a2b8c57e0a407e875 (HEAD -> mojave, origin/mojave)
Author: Ben Selby <ben.selby@emisgroupplc.com>
Date:   Wed Sep 26 20:46:43 2018 +0100

    The start of it working on Mojave

    FIX: Set the correct xcode version for TravisCI

    If we do not set this, we get a warning from TravisCI to explain it doesn’t know how to build the project

    Deal with the swift version and objc inference warnings

See how the extra two commits are now lost. Does it matter?

Ownership

I think it matters, and it revolves around three things, in my opinion.

Technically, I want to know who wrote the code and when. People should get the recognition they deserve, but I also want to know who to speak to 6 months from now when a change is required. If the name next to a line of code is now questionable because of project level squashing of pull requests, I lose that ability to really know who wrote the code. There is, however, a sliding scale of relevancy here, because 18 months from now, that person may have either left the business, or simply cannot remember much context around the code.

Secondly, coding is a social activity in my opinion. People like to express themselves, and that should be encouraged. So if Louise wants to narrate her code and commits in a certain way, she should be free to do that, without a project lead making the ultimate call when merging. Clearly, there are likely to be rules around committing, for example, small, logically atomic commits etc, but within rules there is always flexibility.

Lastly, what if I need to cherry-pick out the TravisCI commit, from the example above, for another branch? I cannot merge the commit into master, as the master branch is not ready for Swift 4.2 (Which is what this commit is telling TravisCI). What if I made a mistake with the “objc inference warnings”? I’m very new to swift, so that’s quite likely, what if I need to get rid of it? I cannot use git to do that now, I would have to code the change.

I’ve spoken to a few people around this, and an argument put forward was “The team own the code when a Pull Request is merged, so it doesn’t matter who has written it”. Again, mixed emotions. I love the buy in from the team in this argument. It is absolutely right that the team own the code, and a statement like that is admirable. But. Erasing recognition of work completed is likely to strain the concept of the team. I have never worked like this, so this is coming from how I perceive the team to behave, not from a history of working like this.

Summary

Right and wrong is subjective. I personally do not like this way of working, but that’s one opinion. I feel too many things are taken away from me as a contributor, and I cannot see any benefit.

Letting people commit their code their way, whilst observing commit message rules etc gives you

See also