6 ms·
> Always use //todo: comments to remind yourself and others about an unfinished job No please don't do this, no one will ever do the todo. The code base I'm cu
by zer0tonin 9y ago
> Always use //todo: comments to remind yourself and others about an unfinished job
No please don't do this, no one will ever do the todo. The code base I'm currently working on has more than 2000 todos.
Instead of this, create a ticket in Jira, Asana or whatever issue tracker you prefer. This way it can be easily taken in account when doing a sprint planning and even if no one opens the problematic file ever again, the issue will still be visible.
- lhnz 9y agoOn the other hand, if you are using `TODO` comments within your code, a good idea is to place something like the `fixme` CLI [0] into a prepush hook [1] so that it is always highly visible. The length of this output then acts as an intuitive measure of accrued technical debt and can be leveraged within a team to get time to fix the underlying problems. [0] https://github.com/JohnPostlethwait/fixme https://github.com/JohnPostlethwait/fixme [1] https://github.com/typicode/husky https://github.com/typicode/husky
- underwater 9y agoA variant I've seen is //TODO(#7362) where the number is a ticket and is enforced by a lint rule.
- marksomnian 9y agoThis is a really good idea.
- sturmen 9y agothis guy gets it
- vahidpg 9y agoThat's a great idea
- CTrom 9y agoThis is what I have found to be a great compromise for my team. Some like todos, others want tickets, so we enforce/support both.
- e12e 9y agoAt least, today, if you want // TODO or // FIXME - do both :add a ticket number, like: // TODO Refactor for Greater Good #123 (or https://<github>|<jira>/#123 https://<github>|<jira>/#123. Filling in issues can feel like "busy work" - but it really is the least bad way to split up work, and keep track of things. I think fossil has the right idea here, distributing issues along with the source code repository (and there's a few attempts to bring that to git/mercurial - but I've yet to find one that really works). In general project "meta data" like documentation and tickets needs to be kept in a database, and there needs to be cross-reference between code revisions and ticket state as well as wiki/documentation - but they probably are 2 or 3 "different" projects.
- Chris2048 9y ago> do both Not all workflows/priorities support this. Sometimes you have windows to meet. Sometimes one task has been authorised, while another has not. Sometimes there is a chance a piece of code will soon become obsolete, so a TODO is conditional on that not being the case.
- ganonm 9y agoThis is misleading advice. Most linters can be configured to fail upon detecting a 'TODO'. I make liberal use of TODOs when developing, typically for reminding myself to e.g. localize a string I've hardcoded during prototyping or to refactor kludgy code. A pre commit git hook gets triggered to run the linter when I attempt to commit which helps avoid the possibility of adding code containing 'TODOs'. Obviously I can just add the '-n' flag to not trigger hooks, but this requires intention. In short, using TODOs appropriately is a massive boost to productivity and the alternative is either to just do those random tasks that pop into your head during the course of writing code or to just hope that you remember to do it later.
- lomnakkus 9y agoI don't think anybody's saying not to do it before merging. (How would anyone even tell?) > A pre commit git hook gets triggered to run the linter[...] Git hooks actually need to be set up to work, so this incurs overhead for onboarding.
- dav 9y agoI built a simple system for some of our repos that fails the build if you don't have the shared team git hooks set up properly, and on failure directs the new engineers on how to fix the situation.
- yes_or_gnome 9y agoThis is one reason why I find GitHub (especially, Enterprise) so frustrating. I want simple server-side githooks to verify that client-side hooks were used (or, at least a good forgery). I don't want to setup an additional CI environment just to run the most basic SED commands. Nor do I want to make my existing CI environment more complicated by merging disparate validations; unit test validation and commit validations.
- dsgriffin 9y agoAlso "unfinished jobs" shouldn't be getting merged in the first place. If you see something unrelated to the feature you're working on in that particular commit, make a ticket on JIRA/Other and address it in a separate commit/PR.
- Kiro 9y agoI prefer TODOs. At the places I've worked filing an issue was a sure way to bury something to never be seen again. You file-and-forget. At one point we even wiped the tracker just because it had so many issues. Definitely a management problem but still a true story. How do you know that a function is unfinished if you have thousands of issues? Adding a TODO puts it in your face so pretty hard to miss.
- taylodl 9y agoThis reminds me of the old saying "if you don't have time to do it now what makes you think you'll ever have the time to do it later?"
- jazoom 9y agoBecause thousands of times in my life I've had more time the day after today.
- Chris2048 9y agoWhat makes you think you ever won't? Plus, sometimes, it might not need to be done tomorrow, if it turns out to be obsolete.
- FullMtlAlcoholc 9y agoBecause I need to push the code out today and I have other obligations to meet in my life.
- chriswarbo 9y agoWhat I need right now might be a working algorithm which gives the right output and gets the job done for the handful of inputs it's being given. What I know right now (since I've just been working on the code) might be a much more efficient way to do the same thing, which requires some tricky implementation details, handling of edge-cases, etc. YAGNI, premature optimisation, etc. says I should stick with the working, slow version until it becomes a bottleneck. The future dev (or future me) who hits a speed issue, does some profiling, and sees this function eating resources, may appreciate finding a comment there listing known issues and potential alternatives. Depending on how bad the performance gets, they may definitely have time to implement the replacement, as they wait for the slow version to finish ;) (Disclaimer: I'm currently trying to speed up an O(n^3) algorithm which was fine for n=200 but is now taking hours for n=3500)
- taylodl 9y agoExactly - YAGNI essentially says you don't need TODOs littering your code. You can cross that bridge if and when you get there. Your disclaimer is interesting. What's more useful than a code base littered with potentially useless TODOs is a codebase containing an explicit set of assumptions agreed upon by the product stakeholders, product owner and implementation team. Then when some aspect of that implementation was influenced by one of those assumptions, say your n=200 and the assumption was n<=500, then you can reference the assumption right there in your code. Then if at some later point in time that assumption should change you can easily find all the code needing to be revisited.
- dspillett 9y agoDo both and reference the ticket in the comment (something like "// TODO: deal properly with NULLs here, see Jira://PRJ-123" with the ticket going into as much extra detail as it relevant). That way someone looking at the code can see that it is not in its intended final form rather than having to intuit the fact from other clues or knowing every ticket in the work list. Particularly useful if trying to find/diagnose a bug that may be caused/worsened/highlighted by the code in question. Also works in reverse: of the ticket doesn't give accurate location information for the related code you can search for the ticket ID and find where you should start looking from the comments.
- andreasklinger 9y agoYour pov is understandable but i treat it differently. They are essentially comments that explain missing functionality. You shouldn't add future features or random ideas but obvious missing functionality or workarounds that happen at this place. i also prefer to leave my name b/c blame never works as it should. it's not the name of the person doing the todo but writing it // todo(andreasklinger): feel free to remove this when the normalization api // workaround (see fileX.js) is no longer needed Related: https://dev.to/andreasklinger/comments-explain-why-not-what-and-2-more-rules-on-writing-good-comments https://dev.to/andreasklinger/comments-explain-why-not-what-...
- EGreg 9y agoHow about, instead of comments, you put function calls to aptply named comments? Comments are, in general a code smell. If a comment is about validation of input or invariants, call a validation function. If a comment is about doing some extra thing, then call a function. This makes you think about how your code is supposed to be structured instead of someone repeating the same code or comment in 1000 places, for example. It's fine for functions to be blank or private but at least we see that there are real pieces of the API missing.
- always_good 9y agoThe problem with this is that you're coming up with an abstraction at a time when you're possibly the least informed about what the abstraction should look like. And then the actual implementor is going to probably roll along with the scaffolding you started. I don't see why people are so hostile towards TODOs. If they don't get done, that sounds like a process problem, altogether something that is going to change project to project, not something to bikeshed on HN every. damn. time.
- EGreg 9y agoI am not just talking about TODOs, but ALL comments. Comments do not use the structure of the language or the IDE to help you. They can't be statically analyzed as easily as the existing syntax. At best, there can be some DSL that is implemented in the comments. I would submit that 99% of your comments can be changed to closures in Javascript. The problem with this is that you're coming up with an abstraction at a time when you're possibly the least informed about what the abstraction should look like. I would say that one of the best times to figure out what the API of the abstraction should look like, is when you're implementing the API of functions that apparently rely on it. You don't need to implement it, but you do need to think about the implementation and API. Otherwise your comments are just wishful thinking, and may not even be implementable. Perhaps the invariants you rely on might be too expensive to compute, for example. When do you think the best time to figure that out is, if not the time when you're writing code that relies on those same invariants? For example a comment like this: function something(platform, options) { if (platform != 'ios') { throw new Error("Not implemented yet folks!"); } doingSomethingOnIos(); // todo: implement for android } should probably instead be replaced with an Adapter pattern where you implement the ios thing now, and you can add an empty android adapter. Much better to assign tasks to developers and have them already know where to fill in the code, AND have an example in your iOS adapter. class Something_Ios extends Something implements Something_Interface { actual tested implementation is here, for others to see } class Something_Android extends Something implements Something_Interface { // TODO: see interface and implement } What's better... the first thing or the second thing? In an ACTUAL PRACTICAL SENSE the second is better. Now the second one may have a "TODO" but the instructions may just as well be in the issue.
- aequitas 9y agoLike said already create a ticket instead of a to-do. But also, instead if writing todos, explain the context and reasoning behind what needs to be done instead. In the future when this code is visited again this will help you or others to get into the right mode to make a proper decision about what to do with this code. Maybe the reason to do something is no longer valid, if the intention is properly documented the decision can be made at that point. This allows for proper refactoring instead of skipping todos and keeping code for legacy reasons. So instead of: //todo: remove this code Do: // required for legacy API clients, should be removed after last client is deprecated.
- Chris2048 9y ago> //todo: remove this code IS fine when the code isn't "finished" within one ticket/cycle etc. After that it needs more detail - However whether there is a ticket or not, I'd still add a TODO (that refers to the ticket id), so that it is more discoverable. There just as great a risk that information gets lost in the ticketing system.
- aequitas 9y agoNothing as permanent as a temporary solution. I've seen countless todos that would be resolved the next sprint/day but never where due to priority shifts or 'it works'. It also doesn't matter wether you loose track of a to-do in your ticket system or your codebase. You will loose it and that can be ok. As long as context and intentions are documented with the code. External or even in repo docs often get forgotten to be updated. Document where it counts.
- Chris2048 9y ago> Nothing as permanent as a temporary solution. Maybe so, but what does "doing them immediately" solve if you're pushed for time anyway? > It also doesn't matter whether you loose track of a to-do in your ticket system or your codebase No, but it does matter if one is more likely. If a TODO is listed right next to the code, anyone who begins working on it has that context immediately.
- saltedmd5 9y agoWhat exactly is an "unfinished job"? If it's functionality that doesn't exist yet, it doesn't need to exist until there is something that requires it, in which case a TODO is useless and your ticket will be a meaningful business requirement if and when that requirement exists. If it's "tech debt," then you have to ask yourself "can it ship like this?" If they answer to that question is "yes," then you should really be asking yourself "does this need doing?" If at that point the answer really is "yes" (unlikely) then hopefully your team has enough context and understanding of their codebase that they can do it when appropriate - if you really really have to, raise a ticket. TODOs are a symptom that something is wrong up the chain.
- dpix 9y agoI came here to write this exact same comment. If people insist on having todos in code. I make sure they at least reference a jira ticket. The code should describe your business logic, not the work to do to build that code. What if no one looks at that file for a year?
- Chris2048 9y agoWhy not put business logic in the ticket too? The think about the TODO is it is more discoverable by devs who are likely to be looking at that code. Nothing wrong with referencing tickets in TODOs, except when references go stale. It's easier to just do periodic 'TODO' searches on mature code.
- always_good 9y agoAgreed. And moreover, the developer already credentializing in a section of code (like working on the router for the past week) is the one that's likely to smash through some outstanding TODOs in that section as well. What doesn't seem to work is to turn every tiny TODO into an issue and then hope someone decides to recredentialize in some possibly large code just to fix some small issues because they stumbled upon it on the issue tracker.
- Chris2048 9y agoIt should also be noted: When working on X, and noticing another piece of work Y, thinking too hard about Y can break context/concentration and your focus on X. Focus needs to be preserved, which is why you drop TODO markers, rather than "just doing them" - you are already doing something, doing something else means not doing what you are meant to be doing now - not pragmatic at all.
- Already__Taken 9y agoThe main issue with that is you can't just clone the repo and walk off with the list of things to work on, you ALSO need the ticketing system constantly accessible and up to date to reflect the code you're working with. but it does seem way neater and easier to use the issue tracking for this purpose, right tool for the job.
- Pigo 9y agoOn a side note, I really can't stand Jira, and don't understand why it's become so popular. Even visual studio online is far less cumbersome, and easier to navigate.
- lucideer 9y ago> No please don't do this, no one will ever do the todo. I've worked in numerous places where TODO placeholders were regularly processed by devs as an explicitly scheduled exercise. If the TODOs are being left to rot, this seems a symptom of bad dev process management, rather than being inherent to the pattern itself. > create a ticket While I've never seen it rigidly enforced, most TODOs I've worked with have been linked to relevant tickets (with ticket #number being referenced in the TODO comment). It's also clearly common enough practice to be supported out-of-the-box without config both by many IDE syntax highlighters and also, I think, by some linters.
- cholantesh 9y agoAlso, it isn't necessary that a ticket creates accountability for the task to be completed. If your process management doesn't include triage, and triage doesn't consider tasks that haven't been touched in awhile, it probably needs some work, too.
- com2kid 9y ago> No please don't do this, no one will ever do the todo. The code base I'm currently working on has more than 2000 todos. Visual Studio integrates TODOs into its UI. After rolling off of feature work I can pull up the TODO list in VS and start picking tasks off, jumping right into the code.
- z3t4 9y agoOr better, don't add more tech debt!! The todo-list will just keep growing and for every todo it will slow down further development. Just do the extra work.
- mmcnl 9y agoAgreed. The more simple answer is to never commit unfinished or broken code. Inline todo's are a definitive no-go. This implies a lack of control of workload management.