No Open Blockers

It’s extremely hard to teach what “good” documentation is. I can try to make suggestions when grading projects, but writing good in-code documentation really just comes with experience. In an effort to expedite that, I try to share certain experiences I’ve come across with my class to make up for their lack of experience with long running projects.

I keep threatening various Spacewalk teammates that I will blog on things they do that were… less than ideal. For instance, I asked one developer what a particular undocumented block of code did. After 10 minutes of staring at it, he still had no idea why he did what he did.

Before I get to any of that, I’ll offer myself up as a sacrifice.

I just got a message from a coworker asking me why I made a particular change to a query. I took a look at the commit, certain that I, in all of my exquisite programming discipline, would have provided a meaningful message as to the change:

“Changed package architecture retrieval to not be a join”

Shit.

A monkey could take about 2 seconds to look at the diff between my revision and the previous and determine I changed the join. The reason this message fails is because I didn’t include why I made the change. Three months after the change, I can’t for the life of me remember what the hell inspired me to make the change, which provides no help whatsoever to the poor sap who is now working with that query.

It’s a good lesson. Good documentation doesn’t just describe what is going on; for the most part you can tell that from the code (depending on the size of the change). Rather, the intention of the code is often particularly important.

For instance, a comment that says “We need the call to flush() to dramatically speed up bulk operations” shows the importance of leaving a seemingly arbitrary call in place (this example is paraphrased, but very real from somewhere in the RHQ server-side code).

Another aspect of good documentation is to describe why something may look odd to someone reviewing the code in the future. Even a comment such as “This should be refactored in the future when we port the other pages to java” is important as it shows the following code isn’t meant to be permanent. It can save a future developer a great deal of time wondering why a particular decision was made by helping to put in context any other factors that were taken into account at the time it was made.

So now that I’ve thrown myself under the bus, I won’t feel guilty pointing out other aspects of Spacewalk that make good examples of what not to do.

UPDATE: To add insult to injury, the fix to that query was committed with a rather descriptive commit message. That’s basically spiking the ball on my lame message.