The Linux codebase has over 3k TODO comments, many from over a decade ago
todos.tickgit.com
todos.tickgit.com
1. It prevents my "flow" from being sidetracked by micro-optimizations that are probably too early to consider necessary anyway.
2. It helps me to retain my short term memory on the code I am working on. If I branched out at each TODO to implement some improvement or method, my brain typically needs to context switch to focus on the fine details of the subject. By the time this is done, and I switch back, I have forgotten key aspects of what I was working on and it slows me down again (i.e. local names, structs, etc.)
3. It allows me to re-approach something with a completely different mind set (given that I come back to it after a signicant amount of times). Half the time I realise that what I wrote was indeed "good enough" and no further time should be committed to it unless a reason exists to do so.
4. It gives opportunity for other developers to see, think, comment and contribute on the subject. I find that typically if I TODO an area, it's good for a second set of eye balls to see it. There's far smarter people than me around, and there's a good chance one of them will find it and propose a better solution.
5. On the rare "quiet" day, I can grep for TODO and just work through them.
Obviously these points are only valid if the TODO labels are being added in situations that will benefit from the above.
RE point 2., it also applies to issue trackers and other "proper" way of encoding TODOs - if I tried to branch out to file a ticket in such situation, or even make a TODO entry in the Org Mode files that always accompany my projects, I'd very quickly lose the flow. Context switch is deadly here.
RE point 5., I try to work through them as I go. I consider this to be a part of cleanup after a main task - I go over all the TODOs in the area I worked in, and implement the simple ones, delete the stale ones, move the serious ones into issue tracker, and leave the rest for future reference.
All of this applies also to FIXME, HACK and NOTE comments - three other types I use. Out of these, NOTE are informative, "seriously please pay attention" comments.
I date them all. I have a Yasnippet for all the above, which expands "todo" into "TODO: $ -- My Name, 2019-12-30", $ being where the caret stops after expansion. Same for "fixme", "hack" and "note".
- FIXME should really not even be committed, except in a proof of concept
- TODO should be fixed eventually, preferably before release to production
- OPTIMIZE is a "nice to have" that gets fixed on a quiet day
Furthermore, TBD (to be done) indicates where new expected functionality is expected to appear, deliberately different from TODO because the JetBrains editor flags TODO / FIXME for confirmation before commit, and will ignore TBD.
In addition to these, I'd like review stubs. Something like:
- REVIEW: can this be done better?
I'd like junior devs to be able to note things as they're thinking about it, not try and remember it later during review. Especially since there may be nothing wrong with the code, it may be as good as it can get[1], so a reviewer wouldn't catch it. Though, I'm mildly concerned about cluttering up code. I suppose you could just remove them on the fly down the line, since they made it past review and etc.
[1]: As good as it can get.. that the reviewer knows of heh
It sounds like the ideal thing to do here, in order to stay in flow, is for these notes to be inline TODOs while coding, but for them to somehow get turned into issues before anyone else sees them. I.e. by a pre-commit hook, or by whatever tooling you use to squash commits when building PRs/patches. If you write them in a standardized format, something should be able to parse them out.
But that would still be very fragile. I don't think there's a good way to record spatio-temporal coordinates of a piece of a file in a way that's resistant to changes. The usual way is to record file position + preceding and following context, but that'll break whenever someone does some bigger changes immediately around the note you're tracking. Perhaps there is some good trick to solve this, but you have to ask yourself - why go to all this ridiculous amounts of trouble to avoid putting TODO notes in comments? It's not like most of them would fit well in an issue tracker either - they tend to be too small or too context-specific to form a nicely packaged unit of work, and making this level of detail visible to managers is just tempting them to cause a disaster for the company.
Why not both? You can refactor todos with a ticket number before you commit the code. So the TODO can have some more context associated with it and maybe a discussion.
why is that necessary ? git blame gives it to you already
But beyond that I agree completely, `git blame` is not the right way to track the authorship of annotations in a file.
One case I've heard is that some people no longer have their names on git commits they made to various Google open source projects because their commit went inside, where it was rebased, merged, integrated, squashed, and then what the public sees is a single git commit by a Google insider that is the result of thousands of individual commits.
git log --first-parent -m
It's a similar reason for why you sometimes leave implementation comments in the code, not in git commit messages.
I'm quite thankful if the code I'm reading is honest and tells me about stuff not up to par with whoever wrote the code/reviewed it expected.
With a TODO it can often be easy to figure out if the feature/bug I'm working on is affected by it.
Potentially a huge time saver.
[0] https://keyboardp.com/post/40190774577/using-task-list-token...
Not sure if it still does this but I found it to be a really good feature.
I found this handy when doing large refactorings (e.g. that took multiple weeks or months). I could define a custom TODO for that refactoring for labeling tasks that I knew I would need to resolve before completing the refactor but was not able to tackle immediately. It made it less stressful knowing I would be easily able to find the tasks again and the central list made it efficient to clear them all.
Until now this approach works quite well for us.
If that’s to cumbersome or difficult, something is wrong in the process or tooling, IMHO.
It makes me go crazy when I find todo scattered everywhere - often these todos are short, hard to decipher and ownership and priorities become jumbled.
What you describe in the points above actually looks like an issue tracker to me.
When I find todos I tend to cut them and paste them as issues trough slack bot integration. It keeps an ok flow, although I get annoyed that my fellows could not be bothered...
To each their own I guess, and whatever works!
Edit: I love in-line comments and documentation though. Just not todos. :)
I’ll look in to setting something up for my team next week... they like todos!
My main gripe with TODO is that they're comments. Comments don't age well. Sometimes they're not written well. I'm sure that a lot of the 3000 TODOs in the Linux kernel are either things that are already done or things that are no longer considered goals. Some of them are surely hard to figure out by now. Personally I only commit a TODO comment in some limited circumstances. Most of the time I make sure that there are not TODO left before I commit. The ones that may be left at the time I want to commit are great candidates for tasks on an agile board or new user stories.
(On the other hand, the bug tracking system is definitely the place to put things which should actually get done.)
1. Write the synopsis of the todo in the code 2. On creation of the MR/PR start discussions on each todo. 3. For each todo which has no quick fix or obvious solution a new issue is created referring to the todo.
This way I get to stay in the flow and record it in a big tracker for further discussion.
To actually enforce this, for the past couple of years I've been using NOCOMMIT comments, together with a git hook that actually prevents me from committing those lines. I rely on this workflow so much now that I can't even imagine going back. I also get really nervous in pair-programming scenarios when someone will say "Yeah, I'll change/fix/adjust that later" - all I'm thinking is "but will you remember?!".
It’s always useful to look at your diffs one last time before adding them to the commit history.
Most of the TODOs I leave are a form of late code review. There’s some sketchy code at the periphery of what I’m working on and I don’t have the time, budget, or inclination to tackle that problem too.
If memory serves, usually for code that won’t respond to a simple refactor. For instance code with complex tests that have locked in the wrong outcomes. Write more two line unit tests, children.
It also help colleagues to discover why you didn't implement things optimally. So they know you were not lazy but responsible.
So you would recommend instead of moving a long he drops what he’s doing and go link the ticket number back into code just to leave a TODO that in high likelihood is him working on eventually or not anyhow? Weird flow but ok.
- 23 from crypto code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
- 2380 from driver code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
- 73 from ARM arch code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
- 43 from x86 arch code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
- 114 from other arch code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
- 606 from block IO, FS, networking, and other sources: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
For example, in much of the database code I work with, TODO is used for optimization banking. In performance critical applications, this is the practice of pervasively tagging (often with TODO) micro-optimization opportunities in the code. When a bug fix or new feature unavoidably creates a significant performance regression (I use 5%), you implement several of the "banked" optimizations documented as TODO in order to bring performance back to parity. For this reason, a TODO may linger in the code base indefinitely and that is okay. It is a searchable annotation for code work that may never need to be done.
You do not put these kinds of TODO items in an issue tracker because they are extremely local and contextual to the code they apply to. Filling the issue tracker with thousands of these kinds of issues which can't be understood without looking at the specific lines of code the comment was applied to has high overhead with negligible value. You do open an issue in the tracker for performance regressions in a subsystem, or if you want to improve the performance by some metric, which directs a developer to start looking at the banked optimizations in the TODO comments inside the code.
Another rarely discussed use case for TODO is assumption verification around the internal behavior of external dependencies. Developers make assumptions about code dependencies (e.g. Linux syscalls or compiler code generation) based on trivially observable and testable behaviors. For code that needs to make strong assumptions about dependency behavior for robustness, it is frequently impractical at the time of writing code to verify that assumptions hold under all conditions or that they will remain true over time (see: Linux fsync() behavior). This becomes an issue that is never "resolveable" in a meaningful sense because it is a moving target and time-consuming to research for any particular case. The TODO is a reminder to re-verify behavior if anything has changed in the software environment or a new bug has shown up and usually notes what has been verified in the past.
Feature and bug work belongs in an issue tracker but there are many "issues", particularly ones that are non-functional and require extremely local context or can never be properly resolved, that are often best served in direct code annotation.
This is one thing that really irks me about most code review cultures I've worked in - they can never stand to see a "naked" TODO (one without a reference to a tracking bug). You're spot on about the result.
The instinct to put a TODO is a note to self that there is a future cleanup, optimization, or improvement. It's a flag to readers of the code that the original author was not quite satisfied with some dimension of a solution. It is _not_ a work ticket, especially in the timeframe that most issue trackers work on (< 1 year).
Maybe there needs to be alternate semantics for these "contextual TODOs", so that people do not immediately get the instinct to either put them in the queue or remove them.
Defense contractor: TODOs don't make it past CR. This is a heavily regulated industry with yearly release cycle
Embedded: TODOs can make it to nightly builds but not final/released build
Web: TODOs are fine, fix them as needed.
It's not at all meant to be to be like an item in a TODO list. Code would be a terrible place to keep that.
The more essential the change/fix, the more hashes. Made it really easy to do a global search for them, and you could easily filter the results to only show the most important ones by searching for longer strings of hashes.
Twenty years later and I’m still doing that in my personal projects.
Using code comments really seems like the wrong place to prioritize changes. Even on personal projects.
A nice cli interface for creating tickets in GitHub/Jira/etc. would help with that. I wonder, would it be nice to have a pre-commit hook that scans for TODO comments, removes them, and drafts tickets? Then both flows would work well.
I linked https://github.com/dspinellis/git-issue in another comment but it gives a sense of the command line experience. I've also at times hacked up various scripts to create or modify items in github/jira/our internal POS, it can make things more likely to hit the tracker than to be met with a shrug. (It's so odd how sensitive programmers can be to even minor blocks in flow like a few seconds waiting for a webpage, or avoiding refactoring a method / class because in Java a new method might best belong to a new class which to do things 'proper' often entails a new file and then another new file for unit tests, adding them to version control, and all that takes like 30 seconds even with IDE support and is annoying.) I've at times thought about a small vim command that would call one of those scripts to create a new issue pre-templated with the file and location I was on, create, then insert the work item id, but never got around to it. As a pre-commit hook something to look for TODOs explicitly might be interesting, though I generally dislike hooks.
It works only for gitlab and assumes that the remote is named origin.
This will create an issue with the title text being everything after TODO. It does not support multiline and only looks at the newly added lines, but does not remove from the commit or alters them.
I personally wanted to contribute to a few projects but it was difficult to track what is currently being worked on or what needs to be done. When an opensource developer only works 8 hours a week on a project you don't want to waste their limited time by having a dozen people ask them what needs to be done and then not doing anything (because a feature might not be relevant to you but you couldn't have known that because it wasn't written down).
It's a poor place for long-term tracking, but it's a great place for recording notes while minimizing context-switching cost while coding.
These should not reach the master codebase and should thus be fixed before you merge your code into it.
Obviously if “next release” is a deploy from master in 2 weeks then it isn’t.
The "accumulation of issues" is there regardless. Leaving TODO's isn't cutting corners or accumulating technical debt. It's simply something that is still to do. Any project will have an issue management system with outstanding issues, but a note in the code is worth more than a thousand words in an issue. "TODO: here is where the validation of parameters must be done, see issue #123". or "TODO: enable right-to-left in these two textboxes when right-to-left text input is added, see issue #234".
There is no value in keeping a main branch "clean" from TODO comments because it's somehow more hygienic. The code needs to be runnable at all times, even with incomplete features. Reaching zero TODO's before a product ships is a matter of grepping for the strings.
If you do stakeholder demonstration from an integrated “develop” branch and ship from master, you can have a policy of “no TODOs in master” meaning you won’t ship the TODOs. I’m not saying one must have TODOs ever in the branch-that-will-be-released but that they can be very useful and almost unavoidable to have in some integrated branch, because you may need to show a product without it necessarily being ready. Exactly what quality gates you ship with is a preference, for example specific comment tags (PERF, FIXME, TODO, ...). These are just words. In my codebase TODO means "this works but isn't optimal, so should be revisited IF the code ever has a reason for change". In other codebases it might mean "This is utterly broken and can't be shipped". Whether shipping is acceptable of course depends on which of those interpretations one uses (And using both would be a process error).
> “this is a thing which is okay during development but absolutely must be changed/fixed before shipping this product”.
There is no valid reason to submit effectively defective code to the main branch. There is a major difference between incomplete functionality, which obviously is what the codebase contains during development, and defective or sub-standard implementation.
As an example (and from one of your examples), leaving validation of inputs "for later" is a sure way of shipping code that does not validate inputs and, again, there is no reason not to properly implement this from the get-go.
This sort of issues only accumulate if you let them by cutting corners.
Real-life example: I saw things like "TODO: Check for null pointers" in code. This should be rejected outright during code review. If you should check for null pointers, then do check for null pointers.
But yes I agree it's a poor example if it's possible to do right away it must be done right away. But I think you get my point. RTL-input may be the better example: it's a nice-to-have for some people, probably not part of an MVP but still a likely part of what some stakeholder calls critical functionality if you support RTL input locales.
My point is that while you may be able to categorize some things as "critical" or "important" (i.e. things one can argue should not reach the mainline even during development) there will always be a gray areas where you can't complete the development, but should still have a functioning app in some sense.
I also use ###. It is just a different style.
I assign the same meaning to TODO or FIXME markers but a lot of people see this is as a measure of poor quality so instead I write regular comments of the form "this could be made better by doing X and Y".
grep -rn TODO *
That's fine, they don't need to be tracked. There are other systems for issues that need tracking.
"Intellj supports it" is a pretty meaningless argument anyway.
I would like to have a tag for "here's something that works, but could be improved" though!
It seems nice to have tags for places to jump where the improvements are obvious, rather than hoping to just remember/spot them later.
Seemed weird to make up a codeword but I wasn't aware of any standard convention, and visually highlighting high impact issues (w/ the bonus of greppability) seemed like a sane approach in rapid development environments.
For personal projects I've used https://github.com/dspinellis/git-issue a few times for a local command line issue tracker, it's not bad. I've got a longstanding mental TODO to try out Fossil one of these days though with all that and more integrated...
HOWEVER: For a mature project, TODOs should be found and reviewed during the pull request. Either they should be fixed prior to merge, or a ticket should be filed. (With a link placed in the TODO comment.)
In the rare case that a TODO remains without a ticket, it really should be something obscure that really doesn't need to be fixed. For example, TODOs are great for micro-optimizations or suggestions for long-term refactoring.
Only so many times can you scroll past a nagging ide hint before you say "FINE, I guess I'll do it now!"
Lowering the barrier to see such comments increases the chances of people actually handling them. Having a highlighted TODO in the code makes it much more visible even if you don't use grep.
Personally I hate TODOs and only see them as temporary things to act as placeholders for the correct code. At some point, I will go through all todos and clear them.
A lot of people use them for things that "could" be done.
The reason I use not just a comment is what others here already said, it gets highlighted by the IDE, and I have too many other "gray" comments that would not get read unless someone is looking at those sections specifically, so another regular comment would be easily overlooked. The suggestion for external tools or files is even worse: Pretty much nobody will check them when it's time for refactoring. It has to be inline. Sure, it can't become too much, but one such "triggerable TODO" comment for just a few modules is fine.
When I write the code I can often already see options to make it more scalable or generalizable, but as it is currently being use the additional cost is not worth it, and if there never is future expansion the code actually is fine as it is. When I already know what needs to be done because my brain is fully 'inside' this code it would be a waste to throw that knowledge away, and regain all the contextual knowledge and ideas with a lot of work a year later. Those are the cases when I add such comments. They are not necessarily "TODO" level at that point, I use that tag only when additional conditions are met and I'm sure I want to avoid missing the comment.
Occasionally, when I have some time over, I scan through my TODOs, there are about 2/1000 LoC. Most of them I leave alone because the condition for their realization has not been met yet, but sometimes I find one of them is worth doing at that point. If it was a general comment I would have no chance to find them that easily. If they were in an external tool or file they would deviate from the actual code more and more, since we do quite a bit of refactoring due to growth and changing needs. But if there's a highlighted TODO in a module you are refactoring it's impossible to miss, so it is easy to update or remove those if they become invalid, even more so than regular "gray" comments.
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).
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.
#if DEBUG
#define TODO(msg)
#else
#define TODO(msg) #error msg
#endif
For anything more long-term you want to use a proper task management system with priorities, deadlines, dependencies and so on.
#if DEBUG
#define TODO(msg) #warning msg
#else
#define TODO(msg) #error msg
#endif #ifdef NDEBUG
#define assert(condition) ((void)0)
#else
#define assert(condition) /*implementation defined*/
#endif
...
assert(("There are five lights", 2 + 2 == 5));
...
test: test.cc:10: int main(): Assertion `((void)"There are five lights", 2+2==5)' failed.
static_assert(sizeof(StructUsedInSomeBinaryProtocol) == SomeConstantExpectedByTheOtherSide, "<...>")And that is ok.
That usually means you add a separate goal and focus to your ticket to be committed inan unrelated issue, which in some projects is frowned upon as it avoids tracking, context, or auditing.
You mean the most successful software project in the history of mankind?
https://www.kernel.org/doc/html/latest/process/2.Process.htm...
Moreover TODOs are added for unmaintained drivers outside of staging/ in order to prepare for their relegation to staging, e.g. see commit a0d58937404f ("PCI: hotplug: Document TODOs").
^-(?!--).*TODO
This should show you all lines removed from source that contain TODO, which should get you in the ballpark.
(Where do you put "TODO: Learn tool X and rework all of this to be completely different using that tool if it makes sense" or even "TODO: The program crashes at shutdown with a double-free, but I don't know which line of code had the extra call to free"?)
*it's fun issuing such rulings.
I agree that closing bugs you don't intend to act on soon is reasonable; I disagree that this makes it not worthwhile to have filed it.
FWIW I also agree that todos specifically about certain lines of code, i.e. todos that will become irrelevant if the code is rewritten at all, should be in code. The limit of that is probably roughly "TODO return the right error subclass". Even "TODO pass more information into this function" is probably past that limit, since it affects at least two spots in the code.
Otherwise issue tracker has thousands of items that the team will never realistically work on.
I work daily on a large, mature code-base, littered with cryptic TODOs left by developers long-gone. They are generally as useful as street-signs in a ghost town.
TODO: clean this up
toDo: validate once ARF-211 is closed
todo remove
TODO factor
TODO: lol, hae to enable in production for some reason
TODO- will this scale? logarithmic?
If I dive into the commit logs, I can try and sus out why they were there, whether they can be removed or acted on. Generally other devs don't touch them out of superstition, they are probably there for a reason, someone else understands them, and they will eventually be useful.
Clearly, better team agreements, discipline, strict code reviews etc could have prevented this or made them more useful, but that ship had sailed before I arrived, and given the pace of maintenance and new feature development I am sure these TODOs will remain for quite some time...monuments to earlier days and priorities that are long gone.
You run into the problem where these todos never get touched because you don't know which are the loadbearing hacks, and tumbling down the rabbit holes won't help you get up to speed.
The only real way to combat this is to dedicate a small team to just start ripping things out/upgrading things/etc.
The only way to clear a minefield is to blow up all the mines....
As far as I can gather, you've described TODOs being useless but harmless
If you're using an issue tracker as well as to-dos in the code, then you've got two issue trackers.
If you're using an issue tracker that also reads to-dos in the code, then you've got one issue tracker.
Where I work we don't have an integration between the tracker and our code, so we disallow to-dos, but allow comments with references to issues. So in other words - one issue tracker.
I can't quite understand why people would want to have multiple places to list code issues.
Jira/Github-like issue trackers can often become blackholes for certain kinda of work. Not all TODO's NEED to get done, they're often just reminders to revisit decisions with the full context of the code. The two most common cases that come to mind are deferred possible optimizations and deferred generalization.
In my workflow, it's more of a counterbalance against premature optimization, abstraction, and distraction. It also has the benefit of:
a) informing reviewers what wasn't done and why right in the diff. It can often spark good conversations of if it's worth scoping them in or maybe scheduling in (Jira!) soon.
b) Allowing people to revisit the above when the next engineer is changing things in the neighborhood.
In my experience, if and engineering side ask isn't addressed in a month or 2 it quickly turns into one ticket that sits in the backlog forever and maybe another that get's created when the "idea" get's floated again.
Naturally, your milage may vary.
That way you always have `// TODO (PJ-1234): better to have some caching mechanism`, where `PJ-1234` is a ticket with "TechnicalDebt" tag, and more details/context.
That gives some transparency to the rest of the team (outside of people working on this specific code base), and avoid situations where nobody knows why a TODO comment exists.
Actual tags are not as important as ability to understand the priority at a glance without looking up a tag in some docs. And fixing all `TODO!*`s is high priority, ideally they should not be committed.
I'm guilty for quite a few
A while ago I started on a tool to automate managing GitHub Issues, but the libraries and tools around issues were mostly crap, so I stopped. But I'd love to get a generic tool together to manage issues using single lines in a text file (such as code comments, or Markdown lists), such that editing a line in the file causes the tool to create, update, close, or delete issues. Infrastructure as Code, but for issues/tickets.
You could then script pre-commit or pre-merge hooks to create issues for TODOs on the fly, and later on have a job skim though code and delete TODO comments whose issues had been closed. Extend it to use Jira and you can use it at work, too.
Most were from over a year ago.
I don't think TODOs are bad practice, and I've seen a diversity of opinions on them in the comments of various posts. I think they are notoriously forgotten, and major codebases like linux and k8s are no different.
Aggressively stale TODOs though are likely an indicator of an area of code that should probably be revisited or cleaned up.
Perhaps your problem is caused by some browser addon.
$ egrep -icr "\bTODO\b" * | tr ':' '\t' > ../todos
$ cat ../todos | awk '{sum+=$2;} END{print sum}'
6372
$ grep "^Docum" ../todos | | awk '{sum+=$2;} END{print sum}'
1172
$ cat ../todos | sort -n -k 2 | tail
Documentation/features/debug/user-ret-profiler/arch-support.txt 24
drivers/media/dvb-core/dvb_ringbuffer.c 26
drivers/platform/chrome/cros_ec_spi.c 33
drivers/media/dvb-frontends/drx39xyj/drxj.c 34
drivers/media/pci/ttpci/av7110_av.c 34
net/ieee802154/nl802154.c 35
drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c 40
drivers/android/binder.c 48
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 51
drivers/net/wireless/broadcom/b43/phy_n.c 54
if (dev->phy.rev >= 19) {
/* TODO */
} ...
static void b43_nphy_tx_cal_radio_setup_rev19(struct b43_wldev *dev)
{
/* TODO */
}1 is immediate, meaning it needs to be fixed for the system to work.
2 is something that should be done before release.
3 is a near term wish list sort of thing, and
4 is a hopeful, probably never sort of thing
In addition, another place I deviate from the norm is when I comment out code, I write the reason why it's commented out.
These two things really help keep my code from becoming incomprehensible as time passes.
FIXMEs : https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
You can either note them in some way as they emerge, or ignore them and keep your code tidy of such notes.
Error: Network error: Origin https://todos.tickgit.com is not allowed by Access-Control-Allow-Origin.
There is very successful company, let's call them "LEG", that makes computer thingies. In order to improve customer confidence, "LEG" has adopted a very strict release policy: all instances of TODO, FIXME, BUG and ERROR are removed before a release.
If you have a variable called "error", automated tools will flag it and you will have to explain to your boss why you could not have used any other name.
You need to make a decision whether to actually fix what you intended to fix or to judge that it's not relevant anymore.
The script is letting you know that there are outstanding things in the code that might have been forgotten. If you are the owner of those TODOs and choose to just delete the comment in order not to have any extra work, then maybe someone in your company should review your presence there.
Are these important issues that should have been fixed or just small good-to-have things? The answer is: yes
There's always stuff that wants doing, but not just now. Except now doesn't come, and those todo notes, like most comments, get ignored forever, peoples' eyes skipping straight over them like they were never there.
https://blogs.msdn.microsoft.com/iliast/2008/05/16/code-qual...
This could make an excellent XKCD.
Edit: to turn this into a more useful comment, let me add that TODO is an important component of test-driven development. If you read Kent's original example with Fibonacci you will note that he splits the functionality into multiple small milestones.But unlikes traditional waterfall these appear organically as TODOs in the test and implementation code as things progress.