Craig Silverstein, 28 September 2018
Note: This internal Khan Academy document was created by Craig Silverstein. It hasn't been posted publicly but as "it's not meant to be sekret" I'm posting it here so I can easily send it to people.
TLDR: the point of TODOs is to provide context for future work.
People use TODOs for more than one purpose: to indicate some functionality is
not fully implemented, to indicate code is messier than it could be, to mark
places where code was not fixed up to match a changing best practice, to
indicate how the code could change to handle increased load, and more.
But each purpose shares a common theme: a decision by the author that the code,
while not perfect, is good enough.
This suggests that TODOs are not tech debt -- if tech debt is defined as code that is not "good enough" according to some criterion -- or, if they are, it's the kind of long-term, low-interest tech debt that fuels productivity. They free a developer up from polishing code more than is needed, so they can work on other things with more benefit to the company. I would argue if you're not leaving TODOs in your code, then you're spending too much time on it.
You may be thinking, "If the author of the code decided it's not worth doing,
why bother writing the TODO at all?" The reason is that while the TODO may not
be worth doing now, it may well be worth doing later, for various reasons.
Perhaps some case that the code doesn't currently handle (as indicated in a
TODO) suddenly becomes important. Perhaps some situation that was unlikely when
the author wrote it becomes more common.
Either way, someone finds themselves looking at code that's no longer working for us. If they find a TODO that captures the problem they're seeing, then they have an instant guidepost as to how they might fix it. This guidepost was written by the original author of the code, who is most familiar with how it works and what its constraints are, and who has often thought fairly deeply about how to do the TODO thing (before deciding it was more work than it was worth). This can be invaluable to someone who is new to the code and scared to make changes to it. TODOs enable you to overcome that fear.
Relatedly, TODOs help you make the "right" change when you're modifying code.
If you're making a change to a file that removes a bunch of TODOs, that is a
good signal you are leaving the code better than you found it. On the other
hand, if you are solving some problem related to a TODO but not resolving the
TODO, that's a hint you should be thinking more carefully if you have the best
solution. (And I should just point out that maybe you do: TODOs are helpful but
they are not law.)
TODOs are also good documentation. You might see some code that implements some functionality in a surprising way, and you're not sure whether or not you're missing some hidden reasoning for it. A TODO can explain why the code is the way it is, and furthermore say how it's supposed to be. (For instance, maybe they couldn't figure out how to make the obvious way efficient, but if they could they'd move to it.) If that "supposed to be" makes sense to you, you can be more confident you actually understand the code you're looking at. If not, it tells you you have more to figure out.
Note that when you write a TODO, you are writing it for a future reader who is
likely visiting the code as a non-expert (to track down a bug or what have you).
You should include as much detail and context in the TODO as you can, to help
people understand it!
The main advantage of TODOs is they are super-low overhead. No context switching is required, there are no particular templates you have to follow, there is no extraneous information you have to include. It is easy to write TODOs without disturbing flow, which in general is very hard to accomplish.
TODOs also benefit by being right next to the code in question, making certain that someone looking at the code will see it.
Like any comment, TODOs can get out of date with the code. This can lead to the opposite of the desired outcome: instead of helping you feel like you understand the code better, they can make you feel you are missing something.
Further, TODOs lack structure: the most common way to track them is via grep.
Our codebase might have 20 TODOs on the same thing -- which could indicate maybe
we should create a jira task to actually resolve it once and for all -- but we
have no idea.
If you are calling some function and you want to know, "Are there any TODOs I should be aware of that might make my function call not work the way I expect?" there's no way to answer that question except tracing through all the code.
Typically, nothing. We don't have an expectation or a requirement that when you see a TODO you drop what you're doing and fix it. If the TODO is related to a change you were planning to make to the code anyway, or if the TODO explains a bug you're trying to resolve, then it's appropriate to deal with, of course.
If you're not sure whether a todo is related to work you're doing, it's probably not. It is not your responsibility to fix a TODO just because you're in the area. Think of it this way: if the original author didn't think it was worth fixing when they wrote it, why should you think it's worth fixing now? If you can't immediately answer that question, that's a sign that you can leave the TODO alone.
If you are seeing a (new) TODO in the context of reviewing someone else's code, then that should be a signal to stop and take a closer look at the approach the review is taking. It may be a sign that another approach would be cleaner. If you, as a reviewer, decide the TODO is the best path forward, also think about whether it rises to the level of deserving a jira task. Your rule of thumb should be "is this something we should resolve soon(ish)?" If so, jira. (And, of course, if the TODO is so important that it should block releasing, it should be a STOPSHIP instead -- those have automated tests to make sure you don't forget about them.)
I appreciate the sentiment. As I mention above, there are different kinds of TODOs that aren't that similar to each other. Shouldn't we use different names for them?
I'd argue no. I want TODOs to be so low-overhead that you don't have to think about whether you should be calling it a TODO or a FIXME or a BUG. And honestly, what does it matter? -- how will readers of the code treat it differently if it's marked TODO vs FIXME? I doubt, in practice, there would be much difference.
If you've created a jira task and want to put in an associated TODO as well, then absolutely! You've done the work to create the task already, you might as well document it better. There is nothing wrong with this belt-and-suspenders approach, and having the link between the two makes it even better. Even if you create the jira task after the TODOs, you may find it useful to have your last step of jira-task creation being to find all relevant TODOs and mark them up with the new task-id.
Having both a jira task and a TODO can sometimes make your life easier. For instance, sometimes it's annoying to try to describe, in the jira task, exactly all the places that are affected by the bug (especially since those places will change as code evolves!). It is often easier to just put TODOs in the relevant parts of the code, that link back to the jira task.
But if your TODO doesn't rise to the level of a P3 or higher bug -- to use an imprecise but useful guidelines -- then it's not necessary. The whole point of the TODO is to be a low-touch way of documenting. Having to create (or reference) a jira task is high-touch: it requires a context switch and a fair bit of time to fill in all the fields, describe context, etc. If you were in flow before you started doing that, you won't be after.
The one thing you are supposed to put in a TODO, besides the actual TODO contents, are your name. (Just to be clear, you do not put the name of the person who you think should fix it, or the team it belongs to; it's just the name of the person who came up with the TODO.) There is (hopefully!) zero overhead required to add that information, and it serves a few useful purposes:
- It tells someone interested in understanding the TODO better, who to talk to, without requiring a bunch of spelunking.
- It gives you some idea who originally wrote the surrounding code, which can be helpful for finding code reviewers even if you don't resolve the TODO.
Often the name next to a TODO is someone who has left the company. That's still
useful information! It tells you that the TODO is (fairly) old. You'll have to
do more work to find out who to ask about it -- usually I do a git blame and
see who reviewed the code who added the TODO, or see who last modified the code
around it -- but them's the breaks.
(While we could always rely on git-blame to find the author of a TODO, it's not
ideal; it's slow, it requires a context switch, and it isn’t always accurate
because it reports who last modified a line, rather than who wrote it. A
formatting refactor, as one example, can cause a line to change in an
"uninteresting" way which git blame can't see through.)
Some teams add other information besides the name: an initiative name, a jira label, etc. If it works for you, that's totally cool. Just put your name in addition.