Squash your Pull Requests!

PRs should include one commit per feature or bugfix – but not more. Especially fix-up commits are really bad and we try to automatically reject them.

rsyslog CI rejects a PR with “bad” structure

A fix-up commit is one that fixes a previous commit within the same PR.The key point is that it does not correct a current coding bug, but one that would have been introduced in the same PR. The proper thing to do is melt it together with the commit that made the mistaken. It is best to not even create the fix-up commit in the first place. Use “git commit –amend” when applying the fix.

There is a hard technical reason why fix-up commits are bad: git bisect provides an easy way to find regressions. When there are commits that do not build (or where tests fail), git bisect does not work.

There are also some soft reasons:

  • they make the commit history harder to read
  • they make it near-to-impossible for e.g. distribution maintainers (read: Debian, Fedora, …) to cherry-pick certain feature
  • last but not least, they create a bad impression on the committer
  • … and also on the maintainer if the PR is accidentally merged without squashing. I admit this happens to me every now and then, as unsquashed PRs are pretty uncommon.

Since 2019-03-03, the rsyslog CI system analyses PR structure and flags potential violations. Note that it is not absolutely necessary to fix these issues, especially if you do not know how to do it. I didn’t know myself for quite some time when I started with git. Thus I can understand that reason. But in that case, the CI fail is visible and warns us maintainers that we need to do the squash during merge.

More in-depth info can be found in the excellent blog post “Always Squash and Rebase your Git Commits”. It also includes instructions on how to do squash and rebase.