18 ms·
The Linux codebase has over 3k TODO comments, many from over a decade ago
- war1025 7y agoIn 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).
- connor4312 7y agoI'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.
- Too 7y agoYep, 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.
- imposterr 7y agoDoesn't that just fall under the "don't let perfect be the enemy of good" ethos?
- juniorplenty 7y agoDisagree. 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.
- fogetti 7y agoI 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?
- Jach 7y agoIf 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.
- ilammy 7y ago> TODO[3 years ago] 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.
- kjs3 7y agoSerious 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?
- nfoz 7y agoI 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.
- kjs3 7y agoInteresting 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.
- wutbrodo 7y ago> 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.
- namirez 7y ago> 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.
- shantly 7y agoIt'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.
- jniedrauer 7y agoHaving 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.
- newnewpdro 7y agoAnd 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!
- war1025 7y agoSome 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.
- alacombe 7y agoEven 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.
- war1025 7y ago> 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.
- thu2111 7y agoWhereas every issue tracker ever has no abandoned or ignored pile of ancient tickets? Must be a nice place to work! Because I've never seen anything like that before.
- geofft 7y agoI 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.
- alacombe 7y agoIssues 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.
- geofft 7y agoNot 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.
- NonEUCitizen 7y agoLet us know when you ship your perfect alternative to Linux.
- war1025 7y agoThat 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.
- wutbrodo 7y agoWhat'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.
- sedatk 7y agoYou 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.
- danShumway 7y agoEh. 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.
- yongjik 7y agoThat'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."
- rumanator 7y ago> 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.
- TeMPOraL 7y agoFrom 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.
- regularfry 7y agoThe 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.
- sedatk 7y agoI 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.
- l0b0 7y agoExactly, 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.
- IggleSniggle 7y agoYou 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."
- im3w1l 7y agoThis may sound weird but I use TODO's to document things I expect to be wonky forever or at least for a very long time. Things I actually intend to fix go in the tracker.
- kaetemi 7y agoYep. 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.
- Forge36 7y agoTODO 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
- JMTQp8lwXL 7y agoThere 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.
- pacifika 7y agoIMHO TODOS help code quality by informing the reader of the weaknesses in the code and considerations with which the code was developed. As a result if it’s out of scope of the ticket then it’s better to have them left in the code then lower code quality by removing them.
- rjsw 7y agoI can see a TODO from 1999 that could be fixed now. There couldn't have been a plan to fix it back then. I don't run Linux myself, I'm not going to bother looking through some bug tracking system. If someone had deleted the TODO then I think I would be less likely to contact the original author.
- iforgotpassword 7y agoTicket 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.
- grumple 7y ago> 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.
- jbaudanza 7y agoI 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.
- Fiaxhs 7y agoCorrect me if I'm wrong but I use "FIXME" for this intent. I keep TODO for stuff actually left to implement.
- kjs3 7y agoThat 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.
- BurningFrog 7y agoTo me, FIX implies that something is wrong, not that it works but could use improvement. I would like to have a tag for "here's something that works, but could be improved" though!
- sombremesa 7y agoPray tell, what line would you NOT put that tag on? That's like, all code...
- kyrra 7y agoChromium has 26,000: https://cs.chromium.org/search/?q=%22//+Todo%22&sq=package:chromium&type=cs https://cs.chromium.org/search/?q=%22//+Todo%22&sq=package:c...
- mcqueenjordan 7y agoWhich makes sense since TODOs tie in to buganizer at Google.
- thrower123 7y agoI'm amazed it is that low. 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.
- tuldia 7y ago> The Linux codebase has over 3k TODO comments, many from over a decade ago And that is ok.
- StuffedParrot 7y agoSure, for the individual. Does the project track them internally? How are they followed up on? How is work assigned and prioritized?
- Forge36 7y agoWhen the file is next edited by the next individual. They don't need to be tracked
- StuffedParrot 7y agoSure, and then you end up with linux.
- NonEUCitizen 7y ago...which is happily used by millions (billions if you include android) getting their work (and play) done in the meantime.
- rumanator 7y ago> Sure, and then you end up with linux. You mean the most successful software project in the history of mankind?
- StuffedParrot 7y agoBy some metrics, sure.
- rumanator 7y ago> 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.
- jcims 7y agoOnly ~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...
- jenkinstrigger 7y agomaybe make a TODO to do it
- Psyladine 7y agoCouldn't you take an older kernel and do a comparison of the TODOs in the most recent, and get a count like that?
- jcims 7y agoThat 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.
- slimsag 7y agoInteresting! I got curious about where they come from, so I dug a little. Here is a non-exhaustive breakdown: - 23 from crypto code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torvalds/linux%24+TODO.*%5C:+case:yes+count:1000+file:crypto/&patternType=regexp https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva... - 2380 from driver code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torvalds/linux%24+TODO.*%5C:+case:yes+count:10000+file:%5Edrivers/&patternType=regexp https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva... - 73 from ARM arch code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torvalds/linux%24+TODO.*%5C:+case:yes+count:10000+file:%5Earch/arm&patternType=regexp https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva... - 43 from x86 arch code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torvalds/linux%24+TODO.*%5C:+case:yes+count:10000+file:%5Earch/x86&patternType=regexp https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva... - 114 from other arch code: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torvalds/linux%24+TODO.*%5C:+case:yes+count:10000+-file:%5Edrivers/+-file:crypto+-file:Documentation+-file:%5Earch/arm+-file:%5Earch/x86+file:%5Earch/&patternType=regexp https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva... - 606 from block IO, FS, networking, and other sources: https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torvalds/linux%24+TODO.*%5C:+case:yes+count:10000+-file:%5Edrivers/+-file:crypto+-file:Documentation+-file:%5Earch/&patternType=regexp https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
- booleanbetrayal 7y agoWithout 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.
- simias 7y agoOr 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.
- shp0ngle 7y agoThe number of FIXMEs seems about half of that, from a quick glance. FIXMEs : https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torvalds/linux%24+FIXME.*%5C:+case:yes+count:1000&patternType=regexp https://sourcegraph.com/search?q=repo:%5Egithub%5C.com/torva...
- modeless 7y agoThe more interesting statistic would be: how many TODOs have been removed from the code, and were they implemented, or just obsolete/wrong?
- dws 7y agoThe codebase that has no TODOs has no vision of its future.
- geofft 7y agoYour 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"?)
- talaketu 7y agoI 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. *it's fun issuing such rulings.
- geofft 7y agoClosed 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.
- james_s_tayler 7y agoIt should just follow a retention policy. If no action on it in the last 12 months just automatically close it. Otherwise issue tracker has thousands of items that the team will never realistically work on.
- katzgrau 7y agoTODO, aka I have the best of intentions but I'm probably not doing this in the near future or ever, for that matter. I'm guilty for quite a few
- john_moscow 7y agoI use this pattern across my codebase for the "critical functionality missing" scenario: #if DEBUG #define TODO(msg) #else #define TODO(msg) #error msg #endif Works like a charm. I've got another PRERELEASE() macro that lets you do an unofficial preview build, but would break an official stable release.
- Toury2d 7y agoThat doesn't sound like it would scale at all for any more than a couple of devs.
- john_moscow 7y agoNeither do TODO comments. This is just for a short-term "don't forget this piece before calling the current job done". For anything more long-term you want to use a proper task management system with priorities, deadlines, dependencies and so on.
- earenndil 7y agoSlight tweak: #if DEBUG #define TODO(msg) #warning msg #else #define TODO(msg) #error msg #endif
- darkkindness 7y agoNice! Isn't this exactly assert in C++? (the following is from https://en.cppreference.com/w/cpp/error/assert https://en.cppreference.com/w/cpp/error/assert) #ifdef NDEBUG #define assert(condition) ((void)0) #else #define assert(condition) /*implementation defined*/ #endif ... assert(("There are five lights", 2 + 2 == 5)); ... test: test.cc:10: int main(): Assertion `((void)"There are five lights", 2+2==5)' failed. Take it as a token that you are doing something right. :)
- john_moscow 7y agoAssert'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: static_assert(sizeof(StructUsedInSomeBinaryProtocol) == SomeConstantExpectedByTheOtherSide, "<...>")
- Jahak 7y agoError: GraphQL error: connection error
- patrickdevivo 7y agoSorry about that! Didn't realize this would hit the frontpage, infra should be upgraded!
- patrickdevivo 7y agoCreator here - this was a total pet project and no idea it'd hit the front page. Upgrading the infra now to handle the load . Sorry for the error rate
- shrimpx 7y agoIt could be a red flag or nothing depending on the development culture around the project and how they assign meaning to these comments.
- patrickdevivo 7y agoFor anyone interested, I wrote a piece last week taking a look at the 2k+ TODOs in the Kubernetes source code as well: https://medium.com/@augmentable/looking-at-kubernetes-2k-todo-comments-b2db42dc7fdb https://medium.com/@augmentable/looking-at-kubernetes-2k-tod... Most were from over a year ago. I don't think TODOs are bad practice, and I've seen a diversity of opinions on them in the comments of various posts. I think they are notoriously forgotten, and major codebases like linux and k8s are no different. Aggressively stale TODOs though are likely an indicator of an area of code that should probably be revisited or cleaned up.
- tomrod 7y agoHow many in the Windows or Mac codebase?
- noisem4ker 7y agoI'd also like to know. It could be a nice addition to this paper, in case of an update: https://blogs.msdn.microsoft.com/iliast/2008/05/16/code-quality-windows-vs-linux-vs-freebsd-vs-solaris/ https://blogs.msdn.microsoft.com/iliast/2008/05/16/code-qual...
- Lammy 7y agoMeta: 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? https://i.imgur.com/QAYtrVB.png https://i.imgur.com/QAYtrVB.png
- xeeeeeeeeeeenu 7y agoOn my end the URL is https://todos.tickgit.com/browse?repo=https://github.com/torvalds/linux https://todos.tickgit.com/browse?repo=https://github.com/tor... Perhaps your problem is caused by some browser addon.
- gotoeleven 7y agoWell hopefully TODO compilation will be in the next C++ standard.
- jameson 7y agolater means never :)
- kitsuac 7y agoMost 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.
- topmonk 7y agoI always write TODO statements with a number from 1 to 4, indicating the severity. 1 is immediate, meaning it needs to be fixed for the system to work. 2 is something that should be done before release. 3 is a near term wish list sort of thing, and 4 is a hopeful, probably never sort of thing In addition, another place I deviate from the norm is when I comment out code, I write the reason why it's commented out. These two things really help keep my code from becoming incomprehensible as time passes.
- nfoz 7y agoThat sort of convention sounds fine for a solo project, but I think if you have more or changing developers then it would make things more confusing.
- mlindner 7y agoI 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.
- james_s_tayler 7y agoI agree but I also think that value is pretty marginal.
- jonny383 7y agoTODO 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.
- pg_is_a_butt 7y ago// TODO!! care.
- TeMPOraL 7y agoIt'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".
- mlang23 7y agoIsn't this a nice case against comments in code? Everything that your compiler can't parse is a waste of time. Where is #todo if you need it?
- randyrand 7y agotodo's are often for future perf or better ways of doing the same thing. Would be nice if todo's were annotated with a category.
- l1k 7y agoFWIW, having a TODO file is mandatory for anything in staging/, see Documentation/process/2.Process.rst. https://www.kernel.org/doc/html/latest/process/2.Process.html#staging-trees https://www.kernel.org/doc/html/latest/process/2.Process.htm... Moreover TODOs are added for unmaintained drivers outside of staging/ in order to prepare for their relegation to staging, e.g. see commit a0d58937404f ("PCI: hotplug: Document TODOs"). https://git.kernel.org/linus/a0d58937404f https://git.kernel.org/linus/a0d58937404f
- bryanrasmussen 7y agoAt 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.
- bryanrasmussen 7y agoFrom 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.
- boyadjian 7y agoTODO is the prerogative of lazy programmers, who wants others to fix their bugs.
- tomhoward 7y agoOr, a way of staying focused on the highest priority work at any given point in time.
- sharma_pradeep 7y agoI have in similar no. o TODO in a 3 yr old codebase. And also 100s of warnings: "Don't touch if it's working"
- dgellow 7y agoA 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.
- buybackoff 7y agoWhen 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.
- jjgreen 7y agoThat's a great idea, I'm going to give it a go myself, thanks!
- keyle 7y agoI guess all the FIXME and HACK have to be addressed first :)
- mastazi 7y agoLink not working for me. I’m getting this: Error: Network error: Origin https://todos.tickgit.com https://todos.tickgit.com is not allowed by Access-Control-Allow-Origin.
- michelb 7y agoI'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.
- known 7y agoI think Students could use them as homework/assignment
- cjfd 7y agoThe '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.
- panpanna 7y agoTODO: 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.
- jbjorge 7y agoIf 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.
- kjeetgill 7y agoIn 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. Naturally, your milage may vary.
- panpanna 7y agoIf 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.
- sweden 7y agoWhich 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.
- panpanna 7y agoLet me clarify: they are not fixing the issues, just hiding them from customers. Are these important issues that should have been fixed or just small good-to-have things? The answer is: yes
- agumonkey 7y agoLet's add one not to forget reducing their amount
- tjpnz 7y agoMight these present an opportunity for those wanting to contribute?
- bythckr 7y ago3k 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?
- sethammons 7y agoI'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.
- jedisct1 7y agoI've never seen "TODO" / "FIXME" sections ever being eventually done/fixed. Never.
- efnx 7y agoThis is why I wrote https://srcoftruth.com 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!
- peterwwillis 7y agoA 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.
- jandrewrogers 7y agoThe 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.
- tomlagier 7y ago
- jwiley 7y agoI 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.
- lucideer 7y agoYour anecdotes are familiar and I completely agree with some of your observations on the limitations of TODOs, but I don't see any reason to be anti- As far as I can gather, you've described TODOs being useless but harmless
- badloginagain 7y agoI'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....
- techolic 7y agoI 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.
- magwa101 7y agoHilarious! 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?"
- Twirrim 7y agoOut of curiosity, I grabbed a copy of the repository and did some basic searching. $ egrep -icr "\bTODO\b" * | tr ':' '\t' > ../todos $ cat ../todos | awk '{sum+=$2;} END{print sum}' 6372 $ grep "^Docum" ../todos | | awk '{sum+=$2;} END{print sum}' 1172 So roughly 18% are in Documentation. The biggest culprit is https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/features/debug/user-ret-profiler/arch-support.txt https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin... , because user-ret-profiler appears to only support x86. All other architectures are in as TODOs $ cat ../todos | sort -n -k 2 | tail Documentation/features/debug/user-ret-profiler/arch-support.txt 24 drivers/media/dvb-core/dvb_ringbuffer.c 26 drivers/platform/chrome/cros_ec_spi.c 33 drivers/media/dvb-frontends/drx39xyj/drxj.c 34 drivers/media/pci/ttpci/av7110_av.c 34 net/ieee802154/nl802154.c 35 drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c 40 drivers/android/binder.c 48 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 51 drivers/net/wireless/broadcom/b43/phy_n.c 54 The biggest single-file culprit for TODOs is in the broadcom b43 drivers, https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/broadcom/b43/phy_n.c 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 */ }