Day 2 - Nuke TODO comments

Congrats on making it to day 2!

Today’s exercise is short and sweet: grep your codebase for TODO comments.

When you find one, I recommend you do one of the following:

  1. Is it out of date? Delete it.

  2. Is it still relevant? Delete it and add it to whatever system you use to track work to be done, like GitHub Issues or Trello or Asana.

  3. Are you unsure? Do a little research and/or track down the comment’s author and get an answer. Then do 1 or 2 :).

Why do this? Because code is a lousy place to track todos. When a todo lives in your code, it can’t be prioritized or scheduled, and tends to get forgotten.

For extra points, submit a PR that deletes all the TODO comments and include links to the newly-created issue for each one. Should make for an easy review/approval.

Worth noting: spend 20 minutes on this task and then declare victory. Success is showing up and putting in the time, not necessarily deleting every comment. If you can only delete one comment in 20 minutes, you’ve still succeeded for the day (well done!)

Finally, when you’re done, please share how old the oldest TODO comment you found.

Get to it!

This task doesn’t go down too well at my work. A particular person likes them since we can easily find WHERE these issues should be changed, but the issue tracker should have a clearer description.

For some reason, we love adding TODO: <Issue Number> into our code base. The oldest issue I got to was Jan 2017, but I also noticed issues that had actually been completed, but the TODO comment not removed - amazing!

Found a total of 126 comments in our main Rails application that had: TODO:, only managed to clear a dozen or so but feeling good about removing the clutter as they added nothing but more lines.

Oldest one I found was from October 2009 that just said: “We’ll fix this later” :joy: :man_facepalming:

whilst looking around also found this gem from a contractor about 5 years ago

“TODO: stab myself in the ******* face for this hack, and then rewrite when we have time”

2 Likes

Decided to spend my 20 minutes to just fix my fav todo comment:

“# TODO make this like 100 times nicer ;-)”

:joy:

1 Like

I’m working on a codebase that is only a few months old so thought I wouldn’t have any TODOs to cleanup but I found five of them in places I never would have thought. All of them were from the first week of development and no longer made any sense in the code so I removed them.

git grep -n -i todo  | wc -l
92

More than I’ll go through today. Let’s see the oldest one:

git grep -n -i todo  | while read line; do 
        file=`echo $line | cut -d: -f1`; 
        ln=`echo $line | cut -d: -f2`; 
        msg=`echo $line | cut -d: -f4`; 
        ts=`git blame -p -L$ln,+1 $file  | grep committer-time | cut -d' ' -f2`
        date=`date -d @$ts`
        echo "$ts $date $msg"
done | sort -n | head -n1

gives us

1340788439 Wed Jun 27 11:13:59 CEST 2012 Check for ajax request

Anyways, removed a bunch of irrelevant todos. One even made it into the issue tracker!

3 Likes

Being the newest member of the team yesterday was really helpful in fixing up the README, but today, I felt like I was lacking a lot of the backstory of the TODOs. Found myself going down a git blame rabbit hole to get context of when they were introduced.

Was able to remove 6! And was encouraged by the fact that the most recent TODO was 9 months ago.

Oldest was from 2013-11-26 (while the ‘initial commit’ was from 2013-05-09).

1 Like

For I knew I was the only one who started adding TODO-flags in the project I was pretty surprised facing quite a lot of them.
I created issues for the topics related and deleted them in the source.
This rose a new challenge: How to deal with changes to the database-object sources if it’s only a change of comments? Deliver it to the customer via migration-files (Pro: Target database and source status are identical, Contra: Huge overhead) or only committing the changes to VCS?
I finally decided to not create migration-files for these changes because I thought the overhead and risk (which is included in every migration) outweighs the benefits of having identical source status. This is because we don’t work with schema-comparison for delivery but rely solely on scripted migrations.
Once the next meaningful change is done to the database-objects, the updated comments will be delivered as well.

1 Like

Done! And I only had to create a couple of Github issues. Most of them are totally irrelevant, outdated, or already done.

I can see the theory that @JalisoCSP’s coworker has - a couple of the Github issues I created now have pretty detailed descriptions of where to find the task.

1 Like

I can see the reasoning, but at the same time, things can be refactored outside of the issue as well and then everything goes to pot.

I think it’s important to make sure minor things just get done then and there, but larger issues should described what it is you’re trying to fix well rather than just where it is.

Wow well done! It’s indeed much more difficult to do when you’re new to the codebase, but I’m sure going through the history and “why is this TODO here” taught you a lot about all this new context.

2 Likes

Here is an easy grep command string that I used:
grep -nr ‘yourString*’ .

The dot at the end searches the current directory. Meaning for each parameter:

-n Show relative line number in the file
‘yourString*’ String for search, followed by a wildcard character
-r Recursively search subdirectories listed
. Directory for search (current directory)

From: https://stackoverflow.com/questions/4121803/how-can-i-use-grep-to-find-a-word-inside-a-folder

1 Like

I had 3 objects that had TODOs: one was mine and the other 2 were code that I got from an outside third party, so the TODO was theirs. In my case, the TODO was more of a note about the timing of processes, so the TODO was more of a comment and not an action item, so I removed the TODO part and left the comment there. That TODO would have been there since about May 2015.

1 Like

January 13, 2011, in a board checkout and initialization routine in U-Boot. This is our own, and ignoring all the TODO and FIXME tags in the U-Boot (and other) mainline.

I’m not having much success REMOVING the TODOS, but it’s definitely bringing me around the codebase on a tour… I just joined the team last week and the worst offender of the TODOs also happens to be their staunchest defender. I’ve already disagreed with him on more than one thing, so I’m trying to take it slow with the opinion havings.

This is my favorite one so far, from last year -

TODO: kill this with fire.

1 Like

For this one I cheated because I remember this being one of the examples @ben mentioned on the podcast and I did it back then. Still, my code gained about 10 TODOs since then. Most of these TODOs were valuable as comments in the code because they were around hacky code, code that should be removed, etc. I wanted to keep the context for whoever is reading the code, so, I added them to Jira and I added the issue number to the TODO. That way, the workflow of fixing them ca be tracked with our Kanban board but the context for someone reading the code remains intact.

Only one TODO! :sunglasses:

I have removed it and created a ticket. I remember that we worked on the fix for the todo but I do not recall why we decided to not do it (the “code template” is there) :thinking:

Oldest was four months ago

Some have lost all context and I was left staring at them and wondering how to convert them to tasks.

Managed to clear them all. :sweat_smile:

I actually like your approach of @TODO <issue #332> If I had more issues, or someone on my team who loves @TODOs, I would do the same thing.

Good exercise!

The oldest TODO in my pet project was just 2015 - and am somewhat embarrassed to discover that I was the author of the most recent one. Shame, shame! In my defence, it is in a feature branch, and hasn’t been committed to master yet…

To folks using grep to look for TODOs - you may want to use the -i option for case insensitive search in order to catch all the permutations of TODO/ToDo/todo/ etc.

You can also use git pickaxe to search for commits that contained (added or removed) a TODO using something like git log -S TODO -i --reverse -p. Explanation: -S is the pickaxe, -i is case insensitive search, --reverse shows you the oldest commits first, and -p shows you the patch of each commit. Bear in mind that this shows you all of the times when a TODO was introduced or removed - so you’ll get some historical information for TODOs long since gone from the codebase.