When working in a team or on an open-source project in a git-centered process, the general advice is to keep commits and pull requests (PRs) clean, small and focused.

The benefits of this advice are numerous:

  • Each commit or PR is small, making it easier and quicker to review.

  • Focused commits or PRs makes it possible to accept good changes quickly while allowing time for discussion about more controversial changes. If all of the changes are bundled together, it becomes harder to separate things.

  • Small, focused commits are easier to understand when looking at past history.

  • Small, focused commits are easier to revert in isolation if a critical issue is discovered.

  • Including semi-related changes such as reformatting or fixing whitespace makes the diffs harder to decipher.

These are important benefits; so much so that they are adopted almost unquestionably by most open source projects and many teams.

Are there any downsides to this advice?

In my opinion there are at least two.

Clean Commits Inhibit Smaller Improvements

The biggest downside I see is that this workflow discourages the kinds of small improvements that we would otherwise make as we go along.

As we’re implementing a change, we might read some related code that doesn’t otherwise need to change. We might notice a name that could be clearer, making the code more understandable next time. We might identify some duplication that should be extracted. We might want to fix some formatting issues.

If we’re following a policy of clean commits, we can’t make these kinds of “drive-by” cleanups because they’ll pollute our commits.

Even something as simple as having our editor set to trim trailing whitespace or insert a trailing newline at the end of the file can violate a clean-commit policy.

If we’re being conscientious, clean commits won’t stop us. We’ll create a new branch, make the changes we identified, and submit them as a separate PR. However, we often don’t feel like we have the time or energy to do this. Be honest with yourself: How many times have you walked away from doing this extra work because it didn’t feel worth it for such a “small” change?

As I’ve discussed before, any obstacles we introduce into our refactoring process will make it less likely that we’ll do the refactoring. Clean commits is one such obstacle.

As the Pragmatic Programmer taught us with the Fix Broken Windows principle, these little messes will compound over time and will ultimately cause us to take less care of our code.

Clean Commits Take Time

The other big downside I see is the time it takes to clean up our commits and PRs before submitting them. Usually, this isn’t much. We need to look over our changes before committing anyway.

But if we want to reorder or reorganize the commits in a PR, we have to take time to do an interactive rebase to clean things up. If we do those all the time, we’re probably pretty quick. But often this can involve going to look up some documentation. And then there are all of the conflicts we need to resolve along the way.

If we’re being extremely careful, we’ll want to be sure that each of the reorganized commits still pass all of the tests. That can take quite a bit of time and care.

Conclusion

I’m not suggesting that we completely abandon a clean commits policy. The benefits are real and important.

I am suggesting that we should pay attention to the downsides and consider whether the policy is delivering the benefits we think it is at an acceptable cost.

Maybe we want to consider altering our policy a bit to allow for drive-by cleanups. Or maybe we decide to be OK with commits that aren’t as well-organized as they could be in order to save time.

The trade-off point might be different for an open-source repository than for an internal project. I suspect the latter might be more tolerant of a relaxed clean commit policy.

As with anything, don’t just adopt a policy blindly; think through the costs and benefits and make a decision based on the balance of trade-offs you see in your environment.