When code has potential issues, I want it to be marked with TODO which basically says "The original developer was not an idiot, but was working with limited resource and the best way to fix the issue wasn't clear then."
I'm sure Google is able to adopt a working ticketing system. If you already have an issue tracker then it makes absolutely no sense to keep a separate out-of-band ticketing repository such as source code sprinkled with TODO entries.
> When code has potential issues, I want it to be marked with TODO which basically says "The original developer was not an idiot, but was working with limited resource and the best way to fix the issue wasn't clear then."
That makes absolutely no sense at all, unless your goal is to simultaneously avoid accountability, poopoo other people's work, and actually do nothing to fix the problem.
The "how do I encode a link to code that doesn't go stale" solution is to use a link to the source code repo browser which points to a specific time and place - don't point at `master`, point at the specific revision hash you're talking about.
As for tracking work, these kind of comment notes are about units of work small enough that tracking them would be very counterproductive for the company. This would be extreme level of micromanagement.
Can you do it immediately? Do it now. Must you do it later? Write it down.
If you decide to use TODO as a way of writing something down then you must make sure that these TODOs are just as visible as whatever ticket system you're using and that they are integrated into your release schedule.
If I'm on another team looking at using your code for whatever reason, I may not be familiar with your ticketing system, what is where, how it's organized and prioritized, etc. I appreciate an indication in the code that that function or class might have some significant areas for improvement. If it's relevant enough to what I'm doing, maybe I'll handle the TODO and open up a code review for your team. Ideally you've got a TODO with some details and also a ticket I can reference to see what's going on with it or what discussion has already occurred around it.
But companies of FAANG size often have so many different developers on totally different teams that they're probably not going to have the time or inclination to go look through your ticket backlog. Having any deficiencies or areas for improvement clearly marked in the code is a benefit in those situations.
We don't need any comments, then, if we have documentation. Right?
My point is they're not the same thing and don't serve the same purpose. A TODO comment is not the same thing as an issue. The audience is, or should be, different, as is the context.
Think of it like those little `print(f"foo is {foo}")` statements you add or the code you comment out while debugging - you use them to quickly develop piece of functionality, then you back them out before `git commit`. This is the point where you'd collect them together and create the tickets - your local work is "done" and you're doing a final review.
Edit: No I thought about it, and I changed my mind again. The TODO comments are primarily for the benefit of the subsequent readers, so they're not having to do so much exegesis and assorted WTFing. If you only have that info in the issue tracker, it's only available to the person dealing with that issue, not the people who have to deal with that code in the course of other maintenance.
TODOs should be an adjunct to the issue tracker. An issue could be "Clean up/resolve all TODOs in module qux". but each of those comments on there own are rarely worth the issue on their own.
Also not every project has the FTEs or the scope to hit every TODO in the first pass. Maybe it's all a glorified demo and these are "Road closed" points to be enhanced/hooked in to in the future.
In my experience, this doesn't work. How many story points does it take to solve _all_ TODOs in a module? How many modules are there? How do I explain this task to a manager or stakeholder? How do you handle TODOs that you think are nice-to-haves or even unnecessary (while your collegue may disagree)? What about TODOs that you do not unserstand (because it is often a one-liner). All those points are mostly solved by 1. Extracting important issues into a ticket OR 2. Commenting the code with a NOTE (instead of TODO), admitting it is not cost-effective to implement. The comment is there as an info for future developers, but it does not look like task that just waits to be implemented.
What I usually do is link the TODO with the associated Github issue/bug tracker (in the commit message too, so it shows up as a reference) to avoid the TODO being forgotten when its blocker is resolved.
See it as an index into the issue tracker so you don't have to search the issue tracker any time you come across some really strange code. This could of course be done with normal comments also, not necessarily todo.
Black-and-white rules like “No TODOs after review!!” are not only too trivial to enforce for a real production team working on deadline, they remove the soft fuzzy subjective edges that make what we do art, not math.
At many companies, issue trackers are not used for things like "refactor this method". They're used for user-facing stories or architectural projects, and they're transparent to managers who don't want them cluttered with non-user facing stories.
I've run into managers who didn't want any TODOs in code, they wanted everything to go through the tracker. I've also run into managers that told me to stop putting everything into the tracker because they didn't know how to prioritize a random method refactor, and they felt like that information wasn't relevant to their scheduling.
I don't have a strong preference, but I lightly lean towards preferring the latter strategy. Issue trackers are slow. They are so slow, and so cumbersome, and so hard to organize, and you waste so much time linking to code files that get refactored or moved around so the context is lost. The nice thing about a TODO in code is if the method gets deleted, the TODO also gets deleted. When you're refactoring code, you don't have to go search an issue tracker and think, "wait, is there anything related to this refactor I need to update?"
If documentation is code that never gets compiled, issue trackers are like dynamically linked libraries that never actually get compiled or linked. It's very hard to keep them up-to-date; it's very hard to preserve the references.
For smaller, single-person independent projects, I don't use issue trackers. I use high-level todo lists, and all of my notes are in code next to the context they're being used for. This is because an issue tracker is just bloat for those kinds of projects.
If you don't have either a united team or managers willing to move at all, or even work with ICs collaboratively, you have bigger problems than what you do with TODO et al. If I were in such a situation I'd still use an 'issue tracker' (maybe a local text file) 9 times out of 10, while also looking for a better job.
Issue trackers are slow and I have to work with one of the slowest (it was built in-house on top of a core product never meant for such a thing...) but some are much faster than others. Command line ones can be pretty slick. Still, it comes down to team (and individual) preference and experience (your "very hard"s are never even "hard" in my experience). Even on a personal project that's going to span a decent length of time, I'll take the slowness of adding even a minor issue and the rare risk of grooming/popping the backlog (and all this might just be in a single local text file, very low overhead) only to discover an item no longer applies and taking the second to Never it. It's not only faster for me in the long run but maximizes the incentives for the things I want in a system that I'm tasked with maintaining and improving.
> If it's not worth of the issue tracker, it shouldn't be worth of a TODO either.
This is what I mean by throwing away work. If you have some insight about the code, your requirement that the insight be thrown away until you can "prioritize, track, and categorize it" makes no sense. Allowing a lighter-weight annotation that lives with the code that it describes allows much more flexibility: it's descriptive of the code and agnostic to its eventual fit into your tasking, as you can integrate it into formally tracked tasks or lump it in with related changes. Blanket bureaucratic rules for bureaucracy's sake is pointless, and I say that as someone who's a very strong believer in ticket tracking hygiene.
The advantage of TODO/XXX/etc. is that you can grep for it. I look for it before committing, vim will syntax-highlight it as an error, etc.
There's no point in grepping for every possible improvement someone might have wanted to do at some point in any part of the codebase. It's much more useful immediately. (I use XXX as a marker to myself for "finish doing this before sending out code for review," specifically because it does get highlighted as unusual in vim.)
If you've got something you want to do in the future, that's totally fine, just don't put it as a code comment. File it in a bugtracker, or if you don't have a bugtracker, put it in a todo file or something and check it in. That lets you at least slice up the work by area of the code (since there is likely no contributor who is equally well-equipped to fix a TODO in any arbitrary spot in the code and equally interested) and by priority and continued relevance. If you have the luxury/curse of doing this for a paying job with scheduled, paid developers, then proper project management will eventually cut the things you never plan to do (or you can use the backlog to decide to hire more developers). Whether or not you do, it's valuable to look at a proper issue tracker, or even a text file, and say "Hm, this thing is so full of unreached improvements that maybe we should flip out and improve it" vs. "You know, this is working fine, it's worth documenting for posterity if someone comes back to this code, but it's not worth specifically calling attention to."
If someone wants to document things about how and why the code was written a certain way - including possible other ways the code could be written, but isn't - a perfectly normal code comment will do the job.
Comment in situ has advantages:
- preserves context without duplicating it into an external system - promotes awareness of the issue when the surrounding code is changed in future, which can lead to serendipitous resolution. - particularly useful when the issue is one of internal quality (eg coupling, duplication, missing test case) that wouldn't ordinarily qualify as a bug or proposed functional enhancement.
In contrast, the bug tracker is a graveyard where such ideas go to die.
If you want the bugtracker or documentation to be checked into the same source repository, that seems entirely reasonable to me. I don't know good tools beyond text files for doing this for bugtracking (although I think Fossil does this, kind of, and I'm sad that Simple Defects never took off), though if your work is uncomplicated enough to go into code comments, it's definitely uncomplicated enough to go into a separate todo file, as I mentioned.
It's very straightforward to do this for documentation. Probably the right thing is to use your existing patch-contribution workflow for this, but make it lighter-weight for docs (reduced or nonexistent code review), but you can also hook up your docs to something like Ikiwiki if you prefer: being in a wiki and being in the source control repo aren't at odds with each other.
And for seeing what was in the dev's mind six months ago - use git log and git blame for that. Even TODOs get refactored away, or moved around enough that they no longer make sense. Good commit messages will show you the whole change that was being made, in context, and what the programmer was thinking when making that change.
At this point, we're discussing between what "should be done" in a perfect world, and what's actually been done when you switch between a couple of project in a day, moving from "implementing feature Y" back to "putting down some random fire", back to "implementing feature X" within a day in an undermanned team. Sometime, a hack will get you back running in prod, and a "TODO" will be there next time you actually have time to address the underlying issue.
If you have a code review step, "please move this comment from the code to the commit message / the bugtracker / whatever" seems like a reasonable comment. If you don't or you're bypassing it or nobody does good code reviews, sure, leave the TODO in. I am indeed talking about the ideal world and whether we think TODO comments are a good idea in the abstract.
I'm afraid if you stick to this principle, you will never have any code ready for review. Using TODO is not an excuse for writing buggy code, but an indication of future improvements and optimizations.
Don't forget Linux is largely a grassroots effort with heaps of reverse-engineered or otherwise improvised/ad-hoc hardware enablement going on. It wasn't until relatively recent history that we could even suspend/resume reliably!
I would bet large money that Linux has a great many of all three of those. They are just handled in a distributed manner by the various teams working on the different features.
Bazaar doesn't mean disorganized. It means there is no central planning authority. There are many individual planning authorities with their own agendas.
Must be a nice place to work! Because I've never seen anything like that before.
Maybe a better question: is there a better way to collect those sorts of low priority tasks?
For larger companies, though, it's really nice to be able to pull up documentation (glorious if you can even use a public Google search instead of some intranet lookup or README) about something, instead of having to go on a safari to try and find someone who knows someone who might remember why X, find out they don't remember (or don't remember enough/don't have time for you until Later), and having to figure it out from scratch yourself like you hoped to avoid. Not having to do that for everything is such a nice experience that I'd like to encourage more of it where I can.
Coming to the specifics you highlighted, "there's probably a better way to do this", "maybe look at this edge case", to me those are topics for the code review. They can be rephrased as questions for the reviewer(s): "Do you think this could be done in a better way?", "Should we try and test this edge case or does it matter?" The answers are in the review, and possibly in follow up work items tracked by something.
Code reviews can contain a lot of useful knowledge. If your commits aren't trivially linked to a review, it's worthwhile to spend the extra 10 seconds and manually link them at the end for posterity. If you aren't doing reviews, or the reviews are crap, oh well, you asked and no one answered/cared and that's now visible. Don't leave the question in the code.
A ticket can better express the problem, so that we can evaluate and triage how serious that problem is, and when the time comes to fix it often we'll come up with a better solution than the TODO would have expressed.
A FIXME is even more obvious: if something needs to be fixed, then let's track an issue/ticket for it, otherwise noone will ever know to fix it.
This seems dramatically more likely when tracked anywhere _but_ the code. As usual, implicit couplings are dangerous in engineering, and are far more likely to fall out of sync when they live far away from the code they're describing.
When the TODO proposal lives with the code, it's a lot easier for discrepancies between the proposal and the current logic to be caught and fixed, whether during implementation, review, or during later reading of the code. None of these possibilities for keeping the comment in sync with the logic arise organically when the proposed improvement is in a bugtracker somewhere, with no pointer from the logic to said tracker.
And as with all advice, it shouldn't be followed blindly.
I mean the best place to communicate important ideas and warnings about code, is the code itself isn't it?
Coming to the project as a new hire, I'd find a "TODO[3 years ago]: maybe add a cache for X to speed things up on this flow" comment less illuminating than "We think this will probably start to fall over at some scale point because we're not using DB connections wisely (and maybe want a cache layer) but our PM didn't give us time [or 'we weren't able to stop the PM shipping before we had time'] to perf test and rework the flow that was patterned after all the other DB-conn-happy logic".
(Not that either comments are that useful -- guess why I'm reading this part of the code?)
But I'm mature enough to think, coming across such code without any of those two comments, that the latter scenario is more likely than "Argh stupid devs, stupid this-dev-in-particular who I can see with git-blame, probably never even thought of a cache! I don't even need to see if there was discussion in the review!" That sort of thought only comes with direct experience dealing with people who repeatedly show themselves as professional failures who should at least retire into management but even better should find a different career field...
Comments like "NOTICE: Don't optimize this loop or you'll introduce a timing attack!" are always welcome in my book. Though if whatever you're warning about can be enforced with a unit test, that's even better insurance against someone naively or even idiotically changing it.
The thing is, you might also be not reading that piece of code for the sake of performance optimization. In this case such TODO is a testimony of premature optimization being evil: like, the code has obviously been fine for at least three years without any smartypants optimization that the developer at that time envisioned to become necessary.
Though yeah, it could have equally been just a 3-year-old ticket in the issue tracker. However, you won't learn about it when casually exploring the code.
Things I actually intend to fix go in the tracker.
For actual immediate tasks, a TODO_123 with the active ticket number is more practical when stubbing some partially written code.
However I still think it's valuable to add TODO and the likes to your code, especially when working on open source, since the issue tracker might not be around forever, you switch platforms, project gets forked, some part reused in another project. It also makes it much easier to get into a new code base, just like comments in general.
Even at work we do this even though using tickets for everything is mandatory. I like it. And in case we open source some stuff folks won't have access to our internal ticket system obviously, so having something more than "see #4642" in your code is good.
I don't run Linux myself, I'm not going to bother looking through some bug tracking system.
If someone had deleted the TODO then I think I would be less likely to contact the original author.
As a result if it’s out of scope of the ticket then it’s better to have them left in the code then lower code quality by removing them.
Your code will probably outlive your ticketing system.
It's also easier to grep for "TODO" than navigate any ticketing system that I've seen. TODO also tells you where the problem exists in code, which is not something that I often see on tickets. Additionally, the non-technical people that handle ticketing within most organizations are unlikely to recognize or prioritize code issues as actual issues. Some organizations might prevent devs from ticketing things themselves as well.
Is the issue still worth fixing? Well the file was completely rewritten 1 year ago, the TODO removed, and no one requesting this now/pushing for it? Close it.
Have an issue but not sure if it's relevant so it stays open forever? I've seen issues outlive the code their for by decades because investigating the lots priority issue wasn't a priority, quick "see: function" references are similar, easily searchable terms which makes old issue review very fast to close. This also ensures the backlog is reviewed periodically
"Many over a decade old" fits pretty directly with what I wrote. TODOs are where good intentions go to die. They get added and rarely addressed or removed.
A TODO item is missing significant business information: date, priority, severity, risk, impact, applicability etc and leaves all those assessments on developer's shoulders who shouldn't have to deal with it at all.
As a matter of fact, every time a developer encounters a TODO around the change he did, it suddenly becomes a burden. He needs to answer "should I be doing this?", "what's the business impact of adopting this?" etc. Even a simple code change can mean business impact in large projects and has regression risks. What if the aforementioned developer isn't at the right caliber for the task, but they don't know it and take it on?
Due to the missing information, it's very hard to analyze TODO data and come up with reasonable engineering decisions too. TL;DR: It makes everyone's job harder.
I only find TODO's suitable for personal projects where you don't want to go back and forth between IDE and the issue tracker.
But a TODO is something different. It's for prototype code, where I don't even know if the feature is going to stick around long enough for the hack to warrant fixing. It says "don't worry about just deleting this whole thing outright and starting fresh." It's graffiti to intentionally make the code uglier, so it's not confused for being part of some larger design. It says, "hey, so, I didn't actually expect this code to survive, but since you're here reading this, it clearly did, you're probably working on something related to it, and you might want to know a few things upfront because it's not going to go the way you think it should go."
- TODO should be a note about behavior.
- TODO should prescribe a small fix.
and yours: TODO should mean "don't worry about deleting this".
These different interpretations and potential confusion they would create make even a stronger argument against TODOs.
In code reviews I always encourage replacing TODOs with a ticket or just removing them. I just can't think of any time when a TODO was actually useful.
Also, the notion that developers shouldn't be involved in "business information" is a non-starter to me. They should absolutely be involved, it's a big part of the job. If they wrote it as a TODO instead of adding something to the issue tracker, they probably thought that it was an issue that did not need to be raised to non-technical people or project managers. You know how managers talk about "managing up" all the time? Dev's do that too.
Edit:
Amending here to avoid replying to ten different threads individually.
1. Long-term improvements should be managed by a ticket tracking system. The code is not the correct place to manage that.
2. Most of the disagreements below seem to come down to nomenclature differences.
In my own work, the code is liberally sprinkled with "Note:"s. These are, as others have pointed out, useful for providing context of what is going on, why, how it is non-optimal, etc.
TODOs that I come across tend to be of the "You ain't gonna need it" variety. If you can get a pull through review as-is, then the thing wasn't actually a "TODO", and if it was, then it should be added as a ticket to the backlog.
XXXs tend to show up in the reviews I do as code that is explicitly meant to be fixed before the pull is merged.
So to summarize:
1. "Note:"s in code are great.
2. "XXX" shouldn't survive the review
3. "TODO" should be handled by the ticketing backlog to better separate out "actually needed" from "theoretically nice, but not necessary in practice".
Further, things that are "TODO" today often make zero sense as "TODO" in a year when the code has grown and evolved more, but since someone put it in as a TODO, I find that they rarely ever get removed since "surely someone knows what that means" (but that person is usually either gone or no longer remembers).