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]

Test-Parallel Development

Here’s a post (albeit dated) where a developer lists a few problems with test driven development. There are plenty more where that came from. What I’ve found works better is a hybrid approach, where we write tests at the same time as code (or just after). The idea behind pure TDD is one of those that sounds good on paper but is difficult to implement practically. Developing to the test means that we abandon some of the best parts of Agile by again tying our hands to strict requirements (this time the requirements are automated tests that don’t work until the code required is implemented). While I am a big supporter of functional automated tests and their inclusion in CI, I don’t think pure TDD is practical. A much better approach is to write functional code and tests together.

The biggest problem I have with TDD is included on the Wikipedia entry on the subject:

Test-driven development is difficult to use in situations where full functional tests are required to determine success or failure. Examples of these are user interfaces, programs that work with databases, and some that depend on specific network configurations. TDD encourages developers to put the minimum amount of code into such modules and to maximize the logic that is in testable library code, using fakes and mocks to represent the outside world.

Fakes and mocks are fine, but I prefer to spend more time implementing tests that run against real world conditions. Also, most all applications that I work on include a UI and/or database. Often, database and UI design occurs alongside all other development.

Taking HTMLUnit as an example, how often do we know what form and input names will appear on a page before we implement it? The same is generally true of database design. In an ideal world, pure TDD would be a great approach. In the real world, where I work, I need more flexibility. This being said, I think most software teams aren’t anywhere near this being a problem. Most have yet to spend appropriate time on automated tests.

Is the Software Medical Device World Ready for Agile?

To begin with, I don’t see any real reason why software medical device manufacturers should fear Agile. I do, however, see some stipulations that need to be made. Here is a rather dated article on the subject (from 2007) : Agile Development in an FDA Regulated Setting.

The author of the blog post concludes:

It seems to me that Agile methodologies have a long way to go before we see them commonly used in medical device software development. I’ve searched around and have found nothing to make me think that there is even a trend in this direction. Maybe it’s that Agile processes are just too new. They seem popular as a presentation topic (I’ve been to several), but I wonder how prevalent Agile is even in mainstream software development?

Since the article was written (4 years ago), Agile has clearly gained a solid foothold in mainstream software development. With companies bound by medical device FDA guidelines, however (or even IEEE, ISO 9001, ISO 13484), there may be some understandable fear on new approaches. What seems to happen is that the known process becomes the only trusted process, and adoption of anything knew leads to so many questions that it is simply pushed aside (regardless of the potential benefit to the company).

The “twelve principles” of the Agile Manifesto include:

  • Customer satisfaction by rapid delivery of useful software
  • Welcome changing requirements, even late in development
  • Working software is delivered frequently (weeks rather than months)
  • Working software is the principal measure of progress
  • Sustainable development, able to maintain a constant pace
  • Close, daily co-operation between business people and developers
  • Face-to-face conversation is the best form of communication (co-location)
  • Projects are built around motivated individuals, who should be trusted
  • Continuous attention to technical excellence and good design
  • Simplicity
  • Self-organizing teams
  • Regular adaptation to changing circumstances

Uh oh. A few of these principles are very likely to send upper management, at least those that are used to their traditional waterfall SOPs, running for the door. But who says we can’t make modifications where we need to?

I suspect that much of the resistance to Agile methodologies is closely tied to a fear of change. Upper management trusts that which they know, despite some of the obvious shortfalls.