Squashing commits
Oct 2, 2018 · 5 min readI 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.
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.
Squashing
Squashing commits is something I do all day long on my own feature branches. I curate 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 curated 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 <[email protected]>
Date: Sat Sep 29 15:59:21 2018 +0100
Deal with the swift version and objc inference warnings
commit 3916021eb03d2c57fcfc412d409985a69494758a
Author: Bob Dylan <[email protected]>
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 <[email protected]>
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 <[email protected]>
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.
First, 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 about 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”. 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.
- The commit history is not cleaner, because when squashing takes place, in the example provided above, the commit messages were not re-written. They were simply squashed together. In the example, two different people wrote the code and the commit messages, so on a squashed merge, the result looks “clunky”
- We lose the ability to see who wrote all of the code
- We lose the ability to understand when the code was written (Late on a Saturday night, or first thing Monday morning?) Does it matter?
- There is a clear record of work undertaken by each member of the team
Letting people commit their code their way, whilst observing commit message rules etc gives you
- A clear commit history. Commits can be very granular, which means it’s easy to cherry pick/revert commits if need be
- A reliable commit history
- Recognition to the people contributing
- In my opinion (subjective here), a more team oriented feeling. A team is made up of many people, who work in different ways