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]

What Every *GOOD* Developer Should Know: Quality Assurance

I’m currently reading “The Clean Coder,” and Robert Martin puts emphasis on the importance of software engineers incorporating QA practices into their regular work much better than I can. Here are a few quotes of his on the subject:

“Software is too complex to create without bugs. Unfortunately that doesn’t let you off the hook. The human body is too complex to understand in its entirety, but doctors still take an oath to do no harm. If they don’t take themselves off a hook like that, how can we?”

“Some folks use QA as the bug catchers. They send them code that they haven’t thoroughly checked. They depend on QA to find the bugs and report them back to the developers.  Indeed, some companies reward QA based on the number of bugs they find. The more bugs, the greater the reward.”

“…So automate your tests. Write unit tests that you can execute on a moment’s notice, and run those tests as often as you can.”

I am really enjoying this book. Its a fun and easy ready.

[Amazon: The Clean Coder]

What Every *Good* Developer Should Know

I came across this guy’s blog post titled “10 Things Every Good Web Developer Should Know.” The post is geared toward web developers, but it did get me thinking a bit about the more general questions. I’ve noticed shortcomings among developers (myself included) for a many years. What are some of the things that all GOOD developers SHOULD be expected to know? This list is hardly comprehensive, but I can think of a few things right away:

1. Linux/Unix

If you can’t do basic editing in vi you may find yourself in for a world of hurt at some point. I’ve known many programmers who attempt to write software in the safety of their IDE running on Windows only to find severe problems when it comes time to deploy on the server (and the server is typically some flavor of Linux). I recall a fellow software engineering student in my college days saying of his code, “It all works, it just won’t compile!” It sounds silly, right? Assuming software that is written, build and deployed in Windows will built and deploy just fine in another OS is equally as silly (yes, this goes for Java as well).

2. How to debug

Duh, right? Not so. I have helped many, many engineers with basic debugging. I don’t know if it is that I am particularly good at debugging (I’d like to think so), or that (some) others are particularly poor at it, but for most of my career I’ve heard, “Matt, this isn’t working, can you help?”

Generally this question is asked by an engineer who has spend a fair amount of time staring at the screen hoping to gain some diving inspiration and fix a bug. It never works this way. You have to be willing to dig into the code and actually find where the error is. Look through that stack trace! Run the debugger! When all else fails, start sticking print lines all over your code! Staring at the screen will rarely reveal a complex bug. A compile error, sure, but a bug, no.

3. Basic knowledge of C/C++ and or Assembly

In the day of virtual machines, powerful IDEs, scripted languages, OOAD and encapsulation on top of encapsulation on top of encapsulation, it can be too easy to write code and never understand exactly how much stuff has to happen for that code to work its magic. I have not written anything in assembly since college, and I have not written C code for 10 years, but I rely on my knowledge of the low-level “stuff” every time I write code. It helps to understand fundamentals of computer science, optimization, memory handling and what exactly makes all the magic of a 4GL come together. Many people get by without knowledge of assembly language, sure, but these people will not be “superstar” engineers… They’ll be programmers.

4. Version Control

There’s no excuse for not using version control. I would say it borders on negligent not to.

5. HTML, CSS, Javascript
This one may seem like another no-brainer, but I have run into many developers over the course of my career who simply do not have anything more than the most basic understanding of HTML.

6. System Administration

Just the other day sendmail quit working for me. I use sendmail to alert the team about project activity in Redmine, Subversion and Jenkins CI. I run project management software that is served using Ruby and Rails, Apache and Tomcat. I have written perl scripts for handling batch jobs and specialized email alerts. I have written bash scripts that tie in to various subversion triggers. I have installed Ant, Maven, Git, Subversion, Tomcat, Apache, GTK, GCC… You name it… All with NO help from a Linux administrator. Like it or not, these activities become the responsibility of the lead software engineer. If you embrace it and enjoy it, life will be easy. If you are lost, and waiting for the help of a system administrator, you may be in for a very long wait!

7. Database Design

EVERY good programmer MUST understand things likes normalization, joins, foreign keys, natural keys, sequences, race conditions, locking, and on and on. We cannot rely on a database designer. Even the largest companies I have worked for have, the ones with database administrators, have little if anything to say about database design. Database design is the responsibility of the software engineer. A poor design can cripple what may otherwise be good software.

8. Quality Assurance

Our goal as engineers it to deliver a high quality product with no defects. We all know that there will be defects, but this fact does not change the goal.

9. Communication, Documentation, Technical Writing

Even if your company does have the means to hire a dedicated technical writer, that employee will have no idea what your code is doing. Strong documentation is on the engineer (us). I never had to take a technical writing class in college. Fortunately, writing is something I enjoy. For the engineer who hopes to never have to write a document, he or she is likely to be very annoyed in this career.

The Clean Coder

I’m reading a book right now titled The Clean Coder. Here’s a quote from chapter 1:

Your career is your responsibility. It is not your employer’s responsibility to make sure you are marketable. It is not your employer’s responsibility to train you, or to send you to conferences, or to buy you books. These things are your responsibility. Woe to the software engineer who entrusts his career to his employer.
-Robert C. Martin, The Clean Coder

[Amazon: The Clean Coder]