TODOs in code

[John Abd-El-Malek]

I wanted to see if we can reach consensus on how TODOs are to be used in the code.

I find that TODOs that don't reference bugs are not very useful, since they're almost always forgotten and just sit there. Creating a bug allows it to be tracked, with people CC'd and hopefully an assigned owner. My experience is that most reviewers do ask for bugs.

We don't have anything about this in our style guide. The Google C++ just says that TODOs are fine, but it's more generic since it doesn't know about bug trackers. Do people agree that we shouldn't have TODOs without bugs?

[Greg Thompson]

I'm with you.

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Ilya Sherman]

I often create TODOs for myself without associated bugs, and follow up on them anyway -- """git gs "TODO(isherman):" | wc -l""" prints "9", most of which are long-term TODOs with bugs filed.  I do not find that filing bugs helps me keep track of TODOs -- it just adds overhead while I wait for code.google.com to do its thing.

Also, in my experience, bugs often "just sit there" as well.  If your goal is to reduce the number of TODOs in the codebase, I'm not convinced that filing bugs for them will help.

~ilya

[Steven Bennetts]

I understand the sentiment here, but I have to agree with Ilya. If we create a policy of requiring a bug associated with a TODO, we will either teach developers to avoid "TODO" in their comments, or we will create additional bugs that remain unresolved. While not always especially helpful, TODO at least indicates that something was intentionally left out / postponed / de-prioritized, and specifies an owner who might be able to help explain why. It can also be a good source of "starter" or "cleanup" tasks.

That said, referencing an issue with a TODO is generally a good idea, and a reviewer should always add their judgement to the contributor's when deciding whether or not the TODO merits creating an issue for it.

--

[Sergey Ulanov]

+1 on this. I often add TODOs for things that are nice to have, but not required, or for trivial fixes (e.g. "remove this code when X is implemented"). Opening bugs for TODOs is useful, but only when it is something that really need to be fixed for specific milestone. If I was required to open a bug for each TODO I would be less likely to add each TODO, which would means the code will be less likely to be fixed in the future.

Codesearch is also useful for searching TODOs you added: http://codesearch.google.com/codesearch#search/&exact_package=chromium&q=TODO%5C(sergeyu%5C)&type=cs 

[Marc-Andre Decoste]

So maybe we would need to do things like "TODO fixit days", or "TODO nagging emails" to help get rid of them sooner than later?

[Jói Sigurðsson]

+1 for the idea of having TODO nag emails.

I think a lot of our TODOs (I know at least some of mine) are really
MAYBES, meaning opportunities for improvement that isn't necessary at
the first go-around but is nice to have documented in a discoverable
form. Should we have MAYBE(username) as a convention in addition to
TODO(username)?

Cheers,
Jói

[Mike Pinkerton]

Rohit had a script that he cobbled together that calculates how many
TODOs each person has in a subtree. Peer pressure always helps, ie,
make sure you're not in the top 10.

Maybe he can dig it up and someone can generalize it?

--
Mike Pinkerton
Mac Weenie
pink...@google.com

[Ben Goodger (Google)]

I like the idea of having a periodic check. When developing new code, TODOs are useful to quickly note areas that are stubbed, and creating bugs for each adds overhead.

But after a while, TODOs can become meaningless.

If there were a monthly nag script, it might promote filing bugs for the legit ones, and removing the obsolete ones.

-Ben

On Tue, Sep 20, 2011 at 6:28 AM, Mike Pinkerton <pink...@chromium.org> wrote:

Rohit had a script that he cobbled together that calculates how many
TODOs each person has in a subtree. Peer pressure always helps, ie,
make sure you're not in the top 10.

Maybe he can dig it up and someone can generalize it?

--

[Evan Martin]

On Mon, Sep 19, 2011 at 7:54 PM, John Abd-El-Malek <j...@chromium.org> wrote:

> I find that TODOs that don't reference bugs are not very useful, since
> they're almost always forgotten and just sit there. Creating a bug allows it
> to be tracked, with people CC'd and hopefully an assigned owner. My
> experience is that most reviewers do ask for bugs.
> We don't have anything about this in our style guide. The Google C++ just
> says that TODOs are fine, but it's more generic since it doesn't know about
> bug trackers. Do people agree that we shouldn't have TODOs without bugs?

There's a flawed premise here, that a filed bug is any more likely to
be resolved than a TODO.

[Peter Kasting]

On Tue, Sep 20, 2011 at 6:52 AM, Ben Goodger (Google) <b...@chromium.org> wrote:

I like the idea of having a periodic check.

Me too.  Like Ilya I use TODOs and bugs distinctly, but there is no reason for a TODO to live forever.

I think it would also be nice to explicitly check for TODOs with no owner name, or owned by someone no longer working on the project.  Both of these should probably be flagged and triaged by the OWNERS of the affected code.

PK 

[John Abd-El-Malek]

In my experience, a bug has some chance of being fixed eventually. A TODO in the code almost never will be looked at.

[James Hawkins]

tfarina cleans up tons of TODOs :-)

[Aaron Boodman]

I sometimes use TODO to indicate something lower priority than a bug,
and dislike when reviewers pester me to create a bugs for these.

For example a piece of code that is non-ideal, but functional. I don't
find that bugs for these kinds of tasks are very useful. They never
rise to a high enough priority to go fish out of the database and
execute on.

But in the code, if someone happens to be there doing something else,
they might be able to act on the TODO at the same time.

Another pet peeve: I dislike the convention of attributing TODOs. Our
codebase is mutually shared, the TODOs should be too.

- a

On Mon, Sep 19, 2011 at 7:54 PM, John Abd-El-Malek <j...@chromium.org> wrote:

[Jonathan Dixon]

On 20 September 2011 20:06, Aaron Boodman <a...@chromium.org> wrote:

I sometimes use TODO to indicate something lower priority than a bug,
and dislike when reviewers pester me to create a bugs for these.

For example a piece of code that is non-ideal, but functional. I don't
find that bugs for these kinds of tasks are very useful. They never
rise to a high enough priority to go fish out of the database and
execute on.

But in the code, if someone happens to be there doing something else,
they might be able to act on the TODO at the same time.

Another pet peeve: I dislike the convention of attributing TODOs. Our
codebase is mutually shared, the TODOs should be too.

Common misconception: as written in the style guide, the name in the TODO is not a mandated owner of the fix, merely the person you should ask for more information about the problem. I think this is very compatible with the opportunistic "future maintainer may fix" it you suggest.

"""

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=TODO_Comments#TODO_Comments

TODOs should include ... the name, e-mail address, or other identifier of the person who can best provide context about the problem referenced by the TODO 

... The main purpose is to have a consistent TODO format that can be searched to find the person who can provide more details upon request. A TODO is not a commitment that the person referenced will fix the problem. 

"""

IMO if we did require all TODOs to have the bug, the bug details would supersede the name on the todo and so make it broadly redundant. But without a bug link, the name is basically a useful short-cut to avoid spending time in git blame.

[John Abd-El-Malek]

to sort of close the loop on this, I don't see a consensus on one particular way, so I will put this topic to sleep.

[Aaron Boodman]

FWIW, I came up with one good thing about using bugs. The bug provides a search string that you can use to find other occurrences of the problem. So I would say that if a TODO has tendrils in more than one spot in the code that you want to call out, it's useful to create a bug and reference that. Not only do you avoid repeating yourself, but when someone eventually fixes the issue, they can find all the occurrences.

- a

[James Hawkins]

I'm curious: how often do you see this happening?

[Paweł Hajdan, Jr.]

Yeah, I've also used and seen this technique several times. I think it's useful and good.

[Aaron Boodman]

How often does a TODO-level problem have tendrils in other areas? I dunno, maybe 2 or 3 times a month. Or roughly half of the TODOs?

- a

[Evan Stade]

besides you can go into a lot more detail in a bug report than a short TODO.

-- Evan Stade

[Scott Byer]

As a reviewer, I don't let a TODO without a bug URL attached go by. It's amazing how much more descriptive the TODO becomes when the description isn't compressed down to a comment.

-Scott