Annotating code with “TODO” can help remind us what code needs attention. It’s easy, however, for them to build up, get out of hand, and no longer be useful.
Below is a system I came up with to keep TODOs useful. Some of what I like about it are that:
- It is based on the basic use of the “todo” string in comments. By retaining the use of the “todo” string, all uses are automatically highlighted for those who environments are set up to by default (including many or most IDEs)
- When searching or filtering, the search or filter will also automatically include higher urgency items.
The plain vanilla TODO, this simply indicates something that should be improved or optimized, but low priority. Fix this if you happen to be working on something related or nearby.
Impact on user experience: minimal to no impact.
- reducing from three non-I/O-bound iterations to one.
- a hacky use of HTML to lay things out
Whether or not this level of TODO is actually useful may vary from team to team. At Goodreads, we have a policy that all TODOs should have a ticket in our bug tracker (more on that below), so the question becomes “If it’s not worth creating a ticket for, there shouldn’t be a TODO here cluttering the code.”
Something that should be improved or optimized, with a bit more urgency
Impact on user experience: theoretically-noticeable (e.g., could impact site performance) to minor impact
- code that should do one bulk operation instead of many single operations
- too much whitespace on the page
Code that I don’t like but has to be that way for now because of time constraints.
Code that needs to be kept around temporarily for compatibility and should be removed/adjusted after the next release (and should have minimal to no impact if it doesn’t get removed right away).
Impact on user experience: May be noticed/experienced or impact performance
- the code lacks graceful error handling (that occurs often enough to matter)
Code that should not go live! I only commit this code when I just want my incomplete work ‘saved’ or I want to commit it so others can work with what I’ve got so far, and:
- I can fix it before it gets released, or
- I know that it will not impact users (i.e., undiscoverable / soft launch) if it ends up going out.
I’ve started writing a new API call, and it will be someone else who finishes it, tests it, and writes the client code that will hit it. Nobody else will hit this code until he’s done, so I’ll mark this code with TODO-XXX, commit it, and let him know to take care of the ‘TODO-XXX’ before it goes live.
IntelliJ can be configured to warn you if you try to commit code containing this pattern (which also includes the next pattern). More on this below.
Code that should not be commited! Use this to mark unfinished code that would impact the user experience or to mark code that you’ve modified temporarily just for development/testing (I also put files with such code into a ‘NO COMMIT’ SVN changelist if it only contains no-commit changes)
Use a pre-commit hook to disallow this pattern from being committed.
Some More Notes
- When searching/filtering, you get higher urgency items for free! e.g., a search for “TODO-XX” will also show you TODO-XXX and TODO-XXXX items.
- While I always check for ‘XXX’ or higher before I commit, I also often go through the ‘XX’ ones, too. “Is there anything I should be taking care of with this commit?”
- For the first three (TODO through TODO-XX) I don’t think it’s easy or valuable to put much effort
into making formal rules — it’s more about how bad you feel about releasing that code =)
- I’ve set up IntelliJ to highlight ‘XXX’ and ‘XXXX’ in comments as colored above (with colored marks in my scrollbar), so that such lines look like compiler/interpreter warnings and errors.
To further abate the number of TODOs growing out of hand, you can also require a TODO to also have a issue-tracker ticket number, and have that be part of the TODO markup. For example, “TODO-512-XX”. So, again, “If it’s not worth creating a ticket for, there shouldn’t be a TODO here cluttering the code.” A pre-commit script could disallow TODOs that don’t have a ticket number in it.
Using this TODO system with IntelliJ
Two easy changes to IntelliJ configuration will allow us to:
- Easily filter for “XXX” TODOs
- be warned when trying to commit files with “TODO-XXX”
- Open up the ‘TODO’ window by press ALT-6 or COMMAND-6.
- Click on the ‘Filter’ icon (which looks like a funnel), and choose “Edit Filters”
- Click ‘Add’ in the ‘Patterns’ panel
- For the ‘Pattern’, enter “XXX+”
- Uncheck “Use color scheme TODO default colors” and choose red or purple for the ‘Foreground’ and ‘Error Stripe Mark’ colors, and then click ‘OK’
- Click ‘Add’ in the ‘Filters’ panel
- For the ‘Name’, enter “XXX” and choose the “XXX+” pattern
Next time in the ‘Commit Changes’ menu, under the “Before Commit” options:
- Check the ‘Check TODO’ checkbox
- Choose the “XXX” filter