TODO Comments have no place in Master


octoDon’t get me wrong, as a professional C# developer I love using TODO markers in my code. They document minor details that I need to revisit and allow me as a coder to keep moving with the aim of getting an overall structure in place. I do this in feature branches and I often do a git rebase -i to tidy up all of my local commits into something more meaningful to merge back into the develop or master branch.

As you are reading this I hope that you will agree with the next few points that I make about source code best practices. So here are my views on what I believe to be a sensible perspective on how you treat releasable code.


Your master branch contains your gold standard source code. You employ a source code management system to look after it and very likely a bug tracking system to monitor and improve its quality. This is the code that gets released. It ships to customers in the form of DLLs, the same DLLs that their developers will routinely disassemble to better understand the APIs that they are working with. From this perspective it should be simple to see that compilable production code should be code that you are prepared to defend in public. It should not contain dead code that has long since been retired. It certainly should not contain any paths used for internal testing or Aide-mémoire TODO Exception classes. These are fairly obvious points.

No here’s the rub. Your developers have likely littered their feature code with TODO comments where they either weren’t quite sure what to do in a specific scenario or simply didn’t have time to flesh out some  minor detail. This is not code that you want to ship to customers, QA maybe, but definitely not the paying public. It follows therefore that you should not be merging this code back into your production master branch. It’s simply not done code. Better to ship without a feature that ship something that you know has holes in it of unknown proportions.

So you have to ship. You have teams of expensive developers churning code at an alarming rate and it all needs to get integrated and fed down the pipe to your QA folks. But there’s these damned TODO comments all over the fresh code. Can I merge these back in? I’d say an emphatic no. If you find yourself in this position you are basically wanting to take on technical debt. This in itself is ok and even sensible on occassion. It is still a debt, and as such the project needs visibility of it. I really love to work with managers who can read code but these are a rarity. If your technical debt is buried away in a source code comment then you can’t really expect your project management to understand and mitigate that debt.

For this reason I would advocate converting any TODO markers into entries in your bug tracking system before this code is allowed to be merged back into your production release stream. Managers understand bug tracking tools, they generate nice reports and illuminating graphs that surface those issues that would otherwise remain hidden in our code. By all means use Git Hooks to police this no TODO comments in your source code attempting to make it into master. This completely bypasses waste with inevitable witch hunts, where you try to track down and re-educate those who merged in technical debt without registering it.

Feel free to leave any feedback or views on this post. The faster that I fail, the quicker I will learn.

image

Advertisement

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s