Appropriate Checkin Comments

I read this post today with a list of funny checkin comments today. Some of them are funny simply because of the lacking description. Here are some comments I’ve seen in my personal experience:

  • many small changes
  • Microsoft IE sucks!
  • cleanup
  • oops
  • fix the bug

Worse, I’ve seen entirely empty changeset comments.

The above lists, along with those found on the funny checkin comments page provide some examples of inappropriate commit comments. Why? They are unprofessional and lacking in detail and meaning. Some projects are audited and reviewed by external third parties. As a project manager or architect, would you be embarrassed for  an auditor to see the comment “fix sucky code?” I would. Even worse than being embarrassed, there is a productivity problem that can arise from poor checkin comments.

What is an appropriate comment? An appropriate comment must (minimally) have a few things:

1. Appropriate level of detail about the change, including why the change was made, what impact there may be, etc.
2. Appropriate to the changeset. Along with this, a single changeset should, as much as possible, reflect a single ticket or change. Many lazy developers check in a large set of code with a number of intertwined changes that are unrelated. When it comes time to revert a particular change or track a defect this creates problems, and ultimately it defeats one major purpose of version control.
3. Details about the completeness of the change. Generally a changeset should complete a ticket or work item, but this is not always the case. If there is remaining work to be done, “TODO” items or further functional requirements that impact the changeset, this should be noted.
4. Finally, perhaps the most important part, the checkin comment should refer to a ticket. Not all changesets have tickets written, sure, but in general, if the ticket is a defect, enhancement or requirement implementation, there should be one or more tickets that are related. Any modern version control and ticket system will be able to tie these together.

One developer writes:

Many developers are sloppy about commenting their changes, and some may feel that commit messages are not needed. Either they consider the changes trivial, or they argue that you can just inspect the revision history to see what was changed. However, the revision history only shows what was actually changed, not what the programmer intended to do, or why the change was made. This can be even more problematic when people don’t do fine-grained commits, but rather submit a week’s worth of changes to multiple modules in one large pile. With a fine-grained revision history, comments can be useful to distinguish trivial from non-trivial changes in the repository. In my opinion, if the changes you made are not important enough to comment on, they probably are not worth committing either.

Getting developers to write good checkin comments seems to be an ongoing battle. In the business of writing software, its easy to convince oneself that checkin comments are a waste of time. The real waste of time is later, when trying to track the introduction of a defect or trace requirement implementation to code. There is simply no good excuse for lacking checkin comments.

Is the checkin comment “cleanup” appropriate? Yes, in some cases, as long as its true. If I am cleaning up formatting of code, including things like indents and spacing and correcting whitespace, then yes, “cleanup” is an appropriate changeset comment. Generally, however, real comments are required.

[Vistamix: The Humor of Code Checkin Comments]
[Loop Label: Best Practices for Version Control]


One thought on “Appropriate Checkin Comments

  1. Hi Matthew,

    From my experience (as a project manager and through the website I manage, PM Hut), you demands from a code comment are quite high. Developers feel that the world owes them something when they add a comment (I’m sure that you know that most don’t). Tying a comment to a ticket will make them avoid commenting altogether.

Comments are closed.