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.
Codemeyer.net 