TODO is vital for my development process. Sure, maybe there's better programmers who don't use or need TODO, but for me, it's a critical method for the following reasons.
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.
It's critical for me too, for exactly the reasons you listed.
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".
I think it makes sense to refine the levels a bit more. We use FIXME, TODO and OPTIMIZE in order of decreasing importance.
- 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.
I was actually just thinking about implementing this type of importance based flags.
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
> 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.
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.
Yes, and it's entirely doable. In case of my own Org Mode notes, I could probably hack together something that does that pretty quickly in both directions (move the note from source file to Org Mode, and display notes from Org Mode inline in the code, at appropriate places). A small alteration could make this work with an issue tracker API.
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.
Beyond it being pretty fragile to have pre-commit hooks edit code, the problem is TODOs are usually very context sensitive. They are usually only understandable given the surrounding code and part of their benefit is they follow refactorings. Tickets wouldn't have that.
> 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.
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.
I might do that after I'm done with the code and about to commit it, but in my experience, there's very little overlap between the kinds of things you'd want to track on your company's issue trackers and the things you want to just jot down inline with the code. The latter tend to be too small.
> 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".
why is that necessary ? git blame gives it to you already
Because running git blame on code breaks the flow. Because git blame may give you wrong author and date (say someone edited the code around the note, changing its indentation level; EDIT: or rebased it, as 'daemin says). Because git blame is not a grep-friendly solution. Because code sometimes lives longer than version control systems (I've worked with codebases older than git). Because very often code lives longer than the repo it originated in, so you can easily lose history (many new projects get started by copy-pasting pieces of a previous project).
I agree with all your other points but I want to point out that if `git blame` breaks your flow I recommend that you look up how to better integrate git with your code editor, it's a huge time saver IMO. With vim-fugitive I can just ":Gblame" on any code I'm currently browsing and immediately I get a side-pane with the annotations for every line. ":Gdiffsplit" shows me the diff between my version and HEAD etc...
But beyond that I agree completely, `git blame` is not the right way to track the authorship of annotations in a file.
It's C-x M-g b in my Emacs, but doing this annotates the whole file, which gets distracting enough for this particular purpose. Still, I use it frequently.
You can rewrite the git history when you squash commits for example.
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.
This is a beautiful example of the general case «why don’t you just use X tool that was designed for this?» but can’t because someone made a decision that makes X tool unusable.
Workflows are full of trade offs. Different people & teams value different features in different ways. It’s what makes software development such a vibrant ecosystem.
Because it becomes more compact. Most of the time git commits are things that don’t make sense to have at a more macro level. Squashing into a single commit that represents a coherent improvement, particularly for large projects, is the best way to read its history.
I prefer a small branch with many individual commits (cleaned from fixups or not) and a merge with main branch which adds the whole feature. Then you can have two levels of reading.
There are many contexts where blame info is not visible by default: dumb editors, code review browsers, search results, etc. It's a low-effort signaling about the age of TODO as well as whether the person who left it is still around.
It's a similar reason for why you sometimes leave implementation comments in the code, not in git commit messages.
That's a fair point regarding visibility. I'll have to reconsider the pluses and minuses because before now I only really saw it as a "too lazy to make a ticket" thing.
Yes. At that point in time, these comments are by definition a list of things that still needs to be done. Removing that will lead to forgetting it, and may create a false impression that the committed code is 100% complete.
Back in my Microsoft development days, I fondly remember Visual Studio automatically adding todo comments to a dedicated little docked window in the corner of the IDE. It was very efficient and would always be in the corner of my eye and I would always circle back to them as a result.
I still use this feature, it comes under the "Task List" window. Adds any "TODO" comments in realtime by default but it's also possible to extend what tokens show up alongside "TODO" [0]. I've added "Hack" or "Perf" for temporary implementations that definitely can be improved but not necessarily critical.
Intellij Idea has this feature as well. Also it checks for new TODO comments when you commit and warns you about them, so you won't forget to fix if you wanted.
It was a while ago since I used Eclipse, but I think it greps all the TODOs in the source code or project workspace, and list them as a list of TODOs :-) for you to work through.
Not sure if it still does this but I found it to be a really good feature.
Eclipse still has this and even better you can define your own tags that are also added to the list view. You can then filter by tag name in the list view.
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.
At work we made it a policy to allow TODOs to remain until code review. At least there, with the help of a second pair of eyes, they should be resolved, removed or made into an issue.
This is a good approach except it breaks the link from the TODO location to the issue. Are you replacing the TODOs with an URL for the issue or something like that? How do you know a file has issues/todos by just looking at the file in your editor/IDE?
Yeah, I dislike banning TODO comments from passing through both for the reason that they're so coupled to the code they reside along that turning them into an issue is rarely helpful, at least as I write them, and that they can convey a lot of information to future readers of the code about intent, about parts of the code the author regarded as shortcuts and maybe a pointer on how to shore them up, about likely shortcomings or gotchas, and so on. Unless you just allow TODO-like comments to remain but without literally having "TODO" in them, which is silly.
It makes a lot of sense if you are only a few people working the code. If not, maybe spend some minutes at the end of the day to register them as issues?
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 use a macro to create an unit test for reason 1 and 2 and name tests with what to test_conditions_expectedResult and add assertTrue(false) as body. I forget to check for todos before I commit, but a failing test is harder to miss
There is alway conceptual TODO at a given stage of development, whether you spell it out or not. If you develop "depth first", these are few and mostly transient, and there is no real benefit to spelling them out. You work through the details before continuing. If you instead program "breadth first" you will ignore many details for the time being and instead queue them up for a future look, and tools like post-its, TODO comments, task management tools become really helpful.
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.
In routine application development (which, to be fair, is a slightly different problem than a kernel,) TODO comments are probably at their best when identifying code deficiencies that you don't actually have plans to fix, pointing out refactoring opportunities, and documenting other hacks. If you put this sort of a thing in a bug tracker or other system, no one will see it when they browse the code.
(On the other hand, the bug tracking system is definitely the place to put things which should actually get done.)
Ideally, every TODO should quickly result in an item on your issue tracking system. The item should point to the TODO in the code so it's easily found. That makes it easy to clean it up once it's not necessary anymore.
Fully agree on the workflow impact. What I usually do is:
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.
Great points, especially 1 - 3. For me it is also often not so much about optimizations, but rather additional use/edge cases that might need consideration, but I'm not sure yet because I haven't e.g. fully fleshed out an interface yet. Often times I will ultimately just delete the note (going back to your third point), but other times it might result in an additional test case, for example. Whenever I'm starting out on a new project (which might just be a new module in an existing code base) my mind tends to bombard me with these hypotheticals, so it is tremendously relaxing to just whip up a quick note and know that it will be dealt with by future me :).
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?!".
Instead of NOCOMMIT, I keep the "is it final" bit in short-term memory (maybe helped by reviewing staged changes) and start the commit message with "WIP:" if necessary. It's a convention from the Qt project that its CI understands. The advantage is that it doesn't block other work until resolved. Sometimes what's missing is even a good/better commit message, which is awkward to remark in the code.
Yes, automatic enforcement is invaluable. Instead of a commit hook you can also use a pre-receive hook on the server. This allows rebasing, wip commits etc. locally and doesn't require any client config.
Your points are valid, and I've used it as well many times, but ther's always the risk you find yourself in a situation where there's just a growing number of TODO items. Which isn't (always) a problem per se, but severely limits there usefulness and could be a sign of ever-expanding problems all over the code base. Worst I've seen is REALLYTODO popping up, then you know the ship is probably beyond sinking. Then again, in principle this is no different from growing todo lists, ever expanding issue tracker issues, ... But at least you can search those for context more easily imo.
To be fair, I believe many projects simply lack the second set of eyes which would downgrade those TODO to basic comments or along the lines of FUTURETODO
One of the more entertaining activities at work is to make fun of ancient TODO comments in our giant codebase. There are some all-stars most of us know by heart such that they made it into the internal company jargon. Tl;dr we ask to resolve TODOs during code review and don't allow them in now.
I try to remove the TODO’s before I commit, or at least before I push changes to master.
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.
TODOs are fine but only if you also have associated cross linked tickets in your bug tracker. You shouldn’t ever commit TODOs in your code without a ticket number or exactly this will happen.
I think you are ignoring all OP’s points about short term memory and not getting side tracked on optimizations. The latter of which gets me all the time.
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.
I used to have a pre-push hook, I think, that would cancel the push if there were any TODOs or FIXMEs in the codebase, displaying them out nicely, with their location. I ultimately turned it off :P
Without auditing, driver code makes sense to me. Guessing there is a lot of vendor churn and therefore a lot of caveats to keep track of ... right up until the point the driver is obsolete and put on life support, never be looked at again.
Or simply the features are never needed so they are never implemented. There's rarely a perfect match between the hardware capabilities and the OS interface, that's the cost of the abstraction. Sometimes there's no trivial way to expose the feature or it would require too much work for something that's not deemed necessary.
Also driver and other hardware-specific code is more likely to have a smaller population of developers to tend and improve it, and a smaller population of users to care about getting it fixed, so minor issues will linger longer. (I had a look at the 'arm' ones, and many of those are in code specific to a particular board or other piece of hardware.)
The kind of software being written materially influences the semantics and use cases for TODO. I think much of the disagreement in this comment thread is based on a tacit assumption that everyone works on the same kinds of code bases. A web app is not the Linux kernel, and the priorities and concerns in development when annotating code will differ.
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.
> 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.
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.
Yes... My own codebase has 669 XXX's, 227 FIXME's and 1564 TODO's... For just 615,981 lines of code. And that's valuable information during development.
I use TODO in my code all the time as shorthand for: "This code is functional, but if you are going to do another iteration you may want to consider the following improvement or optimization."
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 lead programmer at my very first coding job taught me to put a string of at least three hash marks in a comment, to denote “this is a thing which is okay during development but absolutely must be changed/fixed before shipping this product”.
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.
At my work we just file a bug and make sure it gets prioritized as a must-fix... The code might contain a bug number and remark too for redundancy, but the bug itself generally spells out the bits of code that need to be changed/removed/whatever.
Using code comments really seems like the wrong place to prioritize changes. Even on personal projects.
I agree in principal, though if the intent is to have the minimal amount of side-tracking while marking something as TODO for the future, I don’t think you can get better than a comment.
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 suppose it depends on how far you trust your ability to keep everything in your head, and reload everything coming back, and follow through on notes to self. I make todos too, but I try to honor a self-agreement that they be short lived and ideally not make it very far beyond my machine.
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.
If I don't write down everything I will forget what I am working on. Personal projects often get delayed by random things because they don't have a fixed schedule. After a while I forget what I am supposed to work on. What I do instead is I just write an issue with nothing more than a single sentence (often just a title with no body). If a new contributor wants to work on the project then they can use that issue as a starting point to join the project and ask for clarification how I want to it to be implemented.
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 does not make it acceptable, quite the opposite as there would actually be less time pressure to ship code. What you describe would lead to an accumulation of issues over time that will degrade the codebase until it ships and then makes maintenance a nightmare.
A code base that will ship in 2 years will be shipping for various non-production reasons until then. There will be stakeholder demonstrations and alpha releases and things where the code must be functional (runnable), but it may not be production ready in terms of completeness, security etc.
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.
The situation you have described, with there being versions of the code that must not go live, is precisely what branches are for, and branching is cheap. Whether you should consider the particular branch named 'master' to be special in any way, is largely a matter of preference, though if you end up with a mature product, it is quite possible that you will want to be able to cut a release from more than one branch.
Absolutely you can either designate an unstable “develop” branch and do stakeholder demos from there while keeping master clean, or you can develop on master and ship from stabilization branches. You can have any number of branches and assign any level of quality and polices to each. But you can’t avoid TODOs entirely by keeping them only on feature branches as what you’ll need is a branch with all features integrated but not necessarily shippable.
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).
You seem to have somewhat diverged from my point, which was a reply to this statement:
> “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.
Validation may not be possible (Because it requires an external service for example, or validation rules may not even be decided. For example. I suspect that many MVP demos have a login/signup, which might validate the email. I also suspect that e.g. minimum password rules are enforced, it's usually done much later).
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.
> This code is functional, but if you are going to do another iteration you may want to consider the following improvement or optimization.
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".
That may work but I find those comments tend to get lost over time. TODO/XXX/FIXME support are built into IntelliJ which makes stuff like that super easy to find. If some random dev looks and decides its low quality because of those words...well I don't care because its a silly metric to judge on.
I similarly don't want to brag about my skills in pointing out areas of improvement in code that others might think is optimal, so I won't answer that.
Isn't that what NOTE is for? The code is fine for now, but here are some constrains/limitations/implementation details that you should know when you want to touch that par of the code.
That seems like a good compromise. I've used LOOKAT to note places where I got things going but felt like there was a better way of doing things if I had more time.
Is there a standard for valid flags? I recently recommended a client use `TODOSECURITY` for todos which had security implications until fixed - I discovered functionality which was implemented before authorisation had been developed, resulting in a codepath which was unintentionally reachable by standard users.
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.
Yeah, that's probably true. I don't have any visibility over that particular projects ticketing system but I imagine it was never raised given it wasn't fixed until I noticed it.
Might I suggest you give a chance to having separate TODO and WISHLIST and TASKS text files in the root folder that use whatever id scheme you like and point to the code, with the code referring back to those ids only rarely? Or even a single file with sections for those things and more, perhaps called PLAN? Keeping things separate keeps the code tidier, gives you license to put more details than you're likely to put in a one or two line comment, lets you refer to multiple code points instead of the local comment area, can be given to a high school student temp worker, and so forth.
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...
Those things are configurable, though, so new markers are rarely problematic from a tooling perspective. Getting your teammates to agree on using the same markers is perhaps the bigger problem.
Making them agree is not necessarily the hardest. But remembering to tell new team members in a year or two. Or revisiting the codebase in 5 years from now. I doubt you'll remember
I think TODOs are fine for small projects, or early code.
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.
Why would you grep in a scenario where a note is written because "[the]... code is functional, but if you are going to do another iteration you may want to consider the following improvement or optimization.". It's exactly meant for when you encounter it, it's not meant to be found externally.
Because it's trivial to add special highlighting to such notes in most editors and IDEs. Hell, a lot of them come with a configurable list of these comment keywords already built in.
Why would you intentionally leave out a TODO tag to make it harder to find?
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.
TODO, as the name implies, should be things that must be done. Anything aspirational could be left as a mere ordinary comment.
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.
Also, there are things that "must be done" only if the code is extended but it may be perfectly fine as it is at the current state. So I add a TODO when I see that there will probably be the need for refactoring in the intermediate future, but there is no need in the current code base. That is a TODO that is triggered (to become a "true TODO") when that code is touched again, but it remains (justifiably) inactive for now.
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.
yep, TODO is for things like possible optimizations or improvements that are known, and FIXME is for things that are... functioning, but incorrect by some reasonable definition. (YMMV)
In my opinion, any comment prefixed by TODO, XXX, or the like should not survive past the review stage. Either fix the problem immediately or accept that it's going to be wonky forever.
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).
That's a terrible advice. When I worked at Google, many code reviews added more TODOs because the reviewer identified a potential source of problem but also correctly decided that fixing it right there was not the best use of developers' time.
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's a terrible advice. When I worked at Google, many code reviews added more TODOs because the reviewer identified a potential source of problem but also correctly decided that fixing it right there was not the best use of developers' time.
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.
From the point of view of being in the middle of working with some piece of code, it's the issue tracker that's out-of-band! Also, a lot of those TODO entries tend to not be good entries into issue tracker (do you want to track "add extra error checks foobar at line 213 in quux.c"), and they tend to be very localized to the area of code they're left in - which means that, unless your issue tracker supports some way of encoding coordinates in files that survives changes to those files, future people modifying a piece of code are very unlikely to see those notes if they're not left in the code itself.
The counter-argument would be that if it's work, and it needs to be scheduled (which is what we're talking about here) then it needs to be tracked wherever all the rest of the work is, so that it can be made visible and prioritised alongside everything else.
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.
Linking to revision doesn't help in the context of our discussion - it still doesn't solve the problem of automatically getting that information inline with the code at the correct place in the file, for the correct time (if it would show up only in the linked revision then it would be useless; if it would be inherited by future revisions, then how do you delete the annotation once it isn't needed anymore?).
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.
I disagree. The reason those TODOs appear is because the answer to the question "Can you do it immediately?" is, "yes, I can, but it would break my flow and interrupt what I'm doing right now". The reason some of them don't later get promoted to issues in the tracker is because they're too small or otherwise irrelevant at the project level. Their place is in the code, where the next person looking at it will spot them.
Then it is not a TODO, it is a NOTE. Don't act as if it will be _done_ magically in the future. If I stumble upon a TODO, I won't fix it (because it will diasrupt my flow - the reason for the TODO in the first place). And if it is not in the issue tracker, I can't plan for it. A solution would be some kind of an "implement as many TODOs as possible" time-boxed spike, but this never worked really well for me in the past.
>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.
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.
> 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.
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.
I don't think anyone's saying that any time you encounter something which needs fixing you should drop your current work, open up the ticketing system and type out a detailed description there. It's just that the TODO comments shouldn't be part of your final commit (or equivalent).
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.
> 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.
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.
I've been in many scenarios where fixing a certain behavior or implementing something in an optimal way requires a change in an external or upstream product. Or it may require a larger refactoring that's in discussion or otherwise outside the scope of the quick fix. "Either fix the problem immediately or accept that it's going to be wonky forever," sounds like a nice ideal but it's not practical to treat it as an absolute.
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.
Yep, the todo is there to indicate to future code readers that yes we know this sucks but can't fix it properly yet because of blocker X, so don't try to make a half-assed fix yourself without consulting issue tracker nr ### first.
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.
Disagree. There are lots of reasons TODOs live on, mostly having to do with stashing ideas that aren’t functionally critical, or pointers for future devs who might inherit the codebase without immediately grokking a performance optimization or corner case that occurred to the previous owner, but wasn’t important enough to deal with at that time. Sometimes, they just save face for the original dev who would love to make something better, but had to move.
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.
What's the advantage of throwing away context that could be useful in the future? I've taken ownership of codebases with technical debt before, and when modifying the code for something time-sensitive, I often annotate issues with the code whose solutions (usually refactoring) can't be fit into the timeline of the current project. I (or others) can then take advantage of these annotations when I have time to dedicate to refactoring, or in many cases, fit the refactoring into feature work. The fact that a portion of the design groundwork has already been done (as captured by the TODO) makes it easier to fit in the cleanup work. IME, this approach leads to much more rapid improvement of legacy codebases than your description of do-it-now-or-forget-about-it.
You don't throw it away, you put it in the issue tracker where it can be prioritized, tracked and categorized. If it's not worth of the issue tracker, it shouldn't be worth of a TODO either.
Eh. This is a company culture thing that depends on how you use your tracker.
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.
I think it's less of a company culture thing and more of a local team preference thing. If the ICs on a team are united, managers are often willing to change things a bit from how they're used to it. It could be just as simple as prefixing a "[Trust]" tag to items that are important and have some priority within developer minds, but product management has no opinion / ability to form an opinion apart from earlier they've agreed on a general "[Trust]" bucket to account for such things. This lets them be filtered out as needed. Worst case, a united team can have a private issue tracker and beef up their estimates to management to account for work management doesn't want to hear about but needs doing nonetheless.
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.
I don't see why you'd hobble yourself by removing a method of tracking todos, in favor of one that's higher-overhead and more dangerous (much higher chance of getting out of date due to implicit coupling).
> 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.
Coupling it to the code that it affects is nice. If the commit never makes it into master a TODO doesn't either. If the function it's in gets rewritten, the TODO goes away. etc.
I agree with you - specifically because you said "accept that it's going to be wonky forever".
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.
> something you want to do in the future.. don't put it as a code comment. File it in a bugtracker...
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.
Issues bugtrackers get lost, code stay. I prefer to see a TODO and have a glimpse of what was going on in the dev mind (or even my own mind, 6 months ago) rather than "hide this information for the sake of cleanliness." Just the same way, to a large extend, documentation should be in the code, not in a wiki.
Not for the sake of cleanliness - for the sake of storing the information in a useful format.
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.
Most devs I've been working with don't even bother with a full sentence in their commit log.
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.
Oh yeah, a TODO is way better than nothing, no disagreement there.
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.
> TODO, XXX, or the like should not survive past the review stage
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.
It's a great rule if you want to get people to never write down whatever information would have been contained in their TODOs. Anywhere, probably. Best case it remains but just without the TODO label, so... congratulations? Or goes in an issue tracker with all the other junk-issues that no one ever looks at.
Having some way to designate that you're not happy with the code, but it works, is valuable for future refactoring. Another one I see a lot is some variation of "this is a hack." Grepping for "hack" in a codebase can be very entertaining. TODO is more to the point though.
And your kernel would likely never ship, at least not with many drivers anyways.
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!
Some form of tracking tool is the correct place to document the roadmap, in my opinion. The roadmap can be continuously monitored and updated. TODOs in code just end up as comments that exist for a decade because no one along the way wants to delete them, even if they have no relevance anymore.
Even though "mainline" has been structuring a tad over the past 5 years or so, Linux has no roadmap, no product management team scheduling features, no nothing. It is the epitome of bazaar development model done over mailing lists... and it's been working pretty well.
> Linux has no roadmap, no product management team scheduling features, no nothing.
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.
Serious question as someone who drops XXX occasionally: There are all sort of places where you want to note "there's probably a better way to do this" or "maybe look at this edge case" or whatever. Not all of those are important enough to address in the next review/sprint/whatever...why isn't it better to leave them in and keep that tribal knowledge around rather that delete them and loose that insight?
Maybe a better question: is there a better way to collect those sorts of low priority tasks?
I prefer to minimize tribal knowledge of all forms. This does incentivize deletion over "it might be somewhere/in someone's mind", but it also incentivizes moving the knowledge out of the jungle and into something longer lived and searchable in a broader context. How much I prefer the minimization depends on the size/stage of the company, though... At early startups for instance, most tribal knowledge will take care of itself by being forgotten -- and no harm either since the subject matter it applied to is no longer relevant, that was x months ago and nearly everything has changed.
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.
Thanks for taking the time for that detailed response. It seems to me the underlying theme here is "strive for more maturity in the development process". Good lesson.
I think these sorts of notes belong somewhere external of the code itself; an issue ticketing system or whatnot. Because more often than not, I find those "TODO"s lose meaning over time. They often suggest a particular way to solve something, but by the time we might come back to fixing it, that solution is no longer the best way to do it.
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.
> Because more often than not, I find those "TODO"s lose meaning over time. They often suggest a particular way to solve something, but by the time we might come back to fixing it, that solution is no longer the best way to do 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.
I very much agree that if you really want to get to the todos they should be tracked in your issue tracker and prioritized with all other work with the team. What's the alternative? Take a random afternoon to do a random Todo item instead of prioritized work?
Absolutely. That only works in an environment that treats its developers as responsible professionals, not ticket processors, though. If it's going to take a while, just create a ticket while you do it.
Interesting perspective. I don't per se disagree, but my gut feeling (that is, I have no idea if I'm right) is that the farther away from the source code those notes get the less likely they'll ever get looked at much less addressed. I think your view requires more discipline, which, of course, is what we should be striving for.
I am hesitant to write this, because in general I agree, but let's say a product manager stops a feature short of completed in some sense, even though there are some aspects which would make the feature more robust, secure or optimized, the code gets released and one would need a tombstone as a visible nuisance and indicator for these aspects, wouldn't it?
I mean the best place to communicate important ideas and warnings about code, is the code itself isn't it?
If you've let a PM stop a feature short of something you as a dev think is rather important, you've already failed to some extent as a professional. There are ways out of such a mess, but it's better to avoid it to begin with, and if it really couldn't be helped, to have a paper trail if not only for yourself then for the benefit of others.
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.
Yep. Tracker is for stuff that definitely will get planned in. TODO's are just little reminders for when returning to a piece of code some time in the future.
For actual immediate tasks, a TODO_123 with the active ticket number is more practical when stubbing some partially written code.
Ticket systems are great and I use them for pretty much everything I do, and places like github make it very easy to use them even for your small hobby projects.
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.
> 1. Long-term improvements should be managed by a ticket tracking system. The code is not the correct place to manage that.
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.
TODO see issue X is also a great way to link the two together.
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
That is an uncharitable interpretation of what I wrote, and I suspect you know that.
"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.
There are times where there is no review phase. It's a single employee, trying to get something done for their employer in a reasonable amount time, usually limited by a lack of colleagues (not enough engineers) or a rapid delivery timeline. The TODO gets committed, the code is shipped to production, and the TODO comment gets no further attention.
I agree and don't understand people who downvote you as a punishment for expressing your opinion.
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.
You have almost converted me from using TODO. I typically place a "Note:" when there's code that is hacky but not obviously so, to provide context. I also make sure to provide a link if some kind to a relevant 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."
Exactly, TODO comments are basically noise. Nobody goes through code explicitly looking for them, and by the next time someone needs to modify that code the TODO is very likely obsolete or just confusing because of how much the rest of the code and circumstances have changed.
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.
As a developer, seeing a TODO in there is a non-issue. I appreciate the context from the previous developer and I don't feel any sort of pathological urge to stop what I'm doing to fix that TODO (unless it's actually relevant to what I'm doing, in which case, I appreciate that it's there). The way you're phrasing things suggests that you don't trust developers to manage their workload. Is that the case?
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.
Macros cannot expand to preprocessing directives. This does not result in a #error-directive, this just results in a syntax error because of an unexpected # token. A better definition would be something that highlights that message in the error output, such as static_assert(0, msg).
Assert's run-time. This is compile-time (i.e. all instances are guaranteed to trigger during build). C++ equivalent would be a static_assert, but it has a totally different use case like this:
We used to do static asserts with a preprocessor macro that typedefed an array with size expressed as: (your condition) ? 1 : -1. Point being, static asserts have the use case you want them to have; I see no reason to not have a TODO macro that expands to static_assert(false, "...").
> When the file is next edited by the next individual.
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.
Every codebase is like this because it's cheap to open tickets and pepper the codebase with TODOs but orders of magnitude more expensive time-wise to actually do the work. The method of storing the information and prioritizing the work is irrelevant. You simply don't have enough bandwidth to do it all and some of it simply isn't materially important enough to ever get prioritized.
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").
Only ~3,000? Not really that surprising, is it? I think a more interesting metric is how many TODOs have been resolved/removed in that time. I have no interest in figuring out how to figure that out though, so...
That would tell you how many of the TODOs from that specific older kernel have been fixed. But in the intervening releases you could have TODOs come and go.
To find them all you would have to go through pretty much every kernel patch looking for something approximating the following regex (assuming you were looking at TODO comments and not +/- TODO files):
^-(?!--).*TODO
This should show you all lines removed from source that contain TODO, which should get you in the ballpark.
I don't think TODO should be utilized for future functionality but for improvements in existing functionality or things that are missing in the current implementation you can live with short term.
Your vision of the future should be in a bugtracker, not in code comments. TODOs give you a vision of what you would do if you had a few more hours. A bugtracker gives you a vision of what you would do if you had a few more years.
(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"?)
I have completely the opposite opinion! Any bug in the bug tracker hanging around "in a few years" should* be closed. Comments should* persist while the source code issue to which they refer remains remarkable.
Closed bugs are still easily searchable in just about any bugtracking system, whereas deleted code is much harder to dig up (you're less likely to even look for it, and if you do it's harder to find the relevant parts).
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.
I used to be pro-TODO, I find myself today firmly in the no-TODO camp.
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.
I've seen this situation as well. A mature codebase where many of the original authors are gone, with many new devs still getting up to speed.
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....
I understand your point, and I'm probably overreacting, but words like 'pro-TODO', 'no-TODO camp' makes the topic more binary than it deserves, and to some extent urges people to pick sides when it's really not necessary - we're all here to learn something, not to point fingers.
If you don't use an issue tracker, then to-dos is your issue tracker.
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.
In practice they work quite differently in a number of ways, depending on your workflows. I'll outline mine, but from this thread you can probably tell, it's not unique.
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.
A small rule that we set at $WORK, that I found quite helpful for comments: always require a Jira ticket ID if you add a TODO comment.
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.
This is why I wrote https://srcoftruth.com - it will find your todos and open, track and eventually close a ticket for each distinct todo in your repo. Works on GitHub repos and issues (private and public). Currently building JIRA integration and generic web hooks. And it’s free!
I don't understand the hatred for TODO statements (and similar), even if you never come back to them, they're still useful to know the future intention of the original author.
When I see just a simple `TODO` in some OSS project it's often hard to understand either it's a broken edge case or some micro optimization or refactoring left for future. For my projects I adopted (it grew naturally) priority tags, e.g. `TODO!!! [(WTF)]` for critical [tricky/edge case] bugs, `TODO!` for important hot paths optimizations, `TODO (low)` for nice to have, `TODO?` or `TODO (review)` for rethinking design later.
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'll litter my code with TODOs while developing, especially in a green project. However, our team has a practice that we find valuable: any TODO checked into the master branch must contain a link to a jira ticket. A TODO that can't be prioritized is likely to never get done. We like the forcing-feature of making the choice to handle it now, or working to get it prioritized.
A lot of people suggesting "TODOs should become tickets before accepting the code for merge". So... How do you do that? Manually, I assume?
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.
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.
The 'many from over a decade ago' part shows one problem with TODO comments. Very often these TODOs are never done. Really, TODO comments should not exist. When you are writing some code is the point when you know most about it and is therefore also the best point to write code of optimal quality. Also, for some TODOs I encounter in the wild I cannot really shake the impression that the author is showing off by suggesting something that sounds smart but actually is impractical and should never been done.
At a former work we had points for what got added, fixing TODOs was worth 2 points IIRC. We were a small team and people were all good - probably wouldn't work in every situation.
Meta: What's up this the URL on this post? The URL is just to the root of Linus' kernel repo on github, the HN sitebit is 'tickgit.com', but from the comments I figured out the actual submission is supposed to be sourcegraph.com and that nobody else seems to have any issue getting there?
The biggest single-file culprit for TODOs is in the broadcom b43 drivers, https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin.... From what I can clean from phy_n.c, it looks like it's because the linux kernel can't handle revision 19. There's a lot like:
if (dev->phy.rev >= 19) {
/* TODO */
} ...
or
static void b43_nphy_tx_cal_radio_setup_rev19(struct b43_wldev *dev)
{
/* TODO */
}
From my time as a consultant I started doing some TODOs with a view to when I would be off of the project, for example there was code in a place that I knew would be not necessary due to changes at a certain time in the future I would put in a TODO because there was a chance I wouldn't be there when that code needed to be removed or changed. Who knows if anyone ever heeded them though.
Most anything you do, in general, will fork off into many branches. Some being required dependencies and many others being tangential improvements or generalizations.
You can either note them in some way as they emerge, or ignore them and keep your code tidy of such notes.
I'm curious to learn how this is with other large codebases, or operating systems at Apple and Microsoft. I imagine the amount of open bugs or todo's will be much larger, many also from decades ago with no chance of ever being touched.
Hilarious! I remember working on early Unix OS implementations in the 80s. When I saw all the TODOs and NEEDSFIX and WHOKNOWSWHAT I was thinking "How do I make any mods to this?"
If you think that is horrible, let me tell you a story about the opposite approach:
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.
Which makes sense. Why are you making a customer release with outstanding TODOs/FIXMEs/BUGs/Whatever in the code? Are there things left to do? Are you delivering something incomplete to the customer while saying it's bug free and feature complete?
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.
3k TODOs in a vital opensource project. Why? Lack of funding or lack of coders or Linus/Stallman(FSF/GNU, who has resigned but is he replaced?) is unable to be everywhere and he need a bigger team to support him?
What is the "news" here? Is it a warning to avoid Linux's possible "Heartbleed" type issue?
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.
TODO: do something about all those TODOs in our codebase.
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.
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.