Keeping commits focused

March 31, 2014 — Leave a comment

Making small commits often is usually preferable to making massive once-a-day commits – especially if you want someone to review your changes. For me, my stress levels rise with the number of files that have been touched since the last commit. When commits grow too big I’ve been known to stash the code, and redo the changes, breaking up what would have been a large commit into a number of smaller commits. Yes, sometimes large commits are inevitable. They happen. Things could be worse.

By worse, I mean like the two types of commits that have been bothering me on occasion lately.

The first is the one where the commit strays off topic, delving into multiple features at once. For example, you find a commit with the following description:

Added ATOM parsing to RssFeedReader
Validation of ExpirationDate for user account

OK, yeah, those two changes seem totally related. Or maybe they should have been two separate commits? Imagine that we wanted to merge one of these changes to another branch. The Single Responsibility Principle probably applies to commits as well.

However, there is another type that actually bothers me more: The overzealous drive-by code cleaning.

Imagine that you’re looking at the history of the UserAccount.cs file. One of the changes has the following commit message:

Added ATOM parsing to RssFeedReader

Why would RSS parsing affect the UserAccount class? You’re sitting there, scratching your head, wondering why a completely unrelated file has been touched. Upon closer inspection you discover what changed. One line. One unused using statement was removed.

Indeed, that is quite unrelated to adding ATOM parsing to an RSS feed reader. So not only has the history of the UserAccount.cs file been distorted, it was done for pretty much no business value whatsoever.

Now, maybe I’m nitpicking. By all means, clean up the code. But try to keep each commit focused and coherent, instead of going off fixing “problems” in unrelated files. Small fixes like adjusting spacing and removing unused using statements should really only be done if you already had another reason to fiddle with the file. Alternatively, do all the tiny fixes together in one go without changing anything else. Then you can just check the changes in with a suitable comment like “Removing unused using statements”. Accurate, succint and to the point. Clean code. Even cleaner source control history.

No Comments

Be the first to start the conversation!

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.