Clarify conventions regarding TODO comments

[dennwc]

It seems there are no guidelines on how to interpret TODO comments in the code.

To start the discussion, here are some practices I tend to use:

A simple comment looks like this:

// TODO(user): comment text

user is your github username, so people can reach out to you in case they find this TODO while hunting a bug, or if the comment is not clear enough.

For multiline:

// TODO(user): comment text
//             second line

Note that it uses // tokens instead of /* - this is to allow to use /* comment to disable large chunks of code while developing, so comment won't interfere with it. Identation is done this way because editors like Intellij highlight it as a single comment.

There are only two types of comments I use:

• TODO - Something is missing here and is nice to have. Does not affect the current implementation. May be an optimization, cleaner way of doing something, or just a new feature, target for refactoring or future improvement.
• FIXME - A know limitation, a bug or a missing feature that should be fixed and affects the current implementation.

I've seen some conventions for using a Note comments, but I usually omit it and just write a regular comment.

[kuba--]

I like that, but I would even like it much more if I see how it can help in the future. Most likely if you take a look into open source projects it looks like a funny leftovers.

[dennwc]

At least for me, I always try to analyze edge cases and failure modes, and sometimes there is not enough time to cover everything, or current use case does not require it. So I usually place TODO to remember what kind of use case and/or input I've considered, and it helps later if the use cases changes and assumptions become invalid.

And you feel a bit of a relief when hunting a bug leads you to your own TODO left a year ago - this way you know it was an expected failure, not a programming mistake (it's a feature, not a bug! :grin:).

[juanjux]

While writing the user is not bad by itself, you're only one git blame away from seeing who wrote it without it.

[dennwc]

Sometimes git blame may show the user who fixed indentation, copied the file or code snippet. User name is more reliable in those scenarios.

[creachadair]

I generally use FIXME comments as something to grep for when I'm working, and ideally those should never get committed (except locally in a dev branch).

As far as TODO comments, I generally agree that they should be qualified: Either the person who should be consulted, or better still TODO(#xxx) with an issue number that contains the context for the discussion. It's really easy for individuals to forget what they were thinking, and an issue is a nice updateable history that people can add to over time.

In any case, I don't think the primary virtue of any such comment is in the history: git blame is limited, but Git itself has enough data to figure out who wrote and/or edited a line. Much more interesting is the context that the code doesn't contain: Why did we do it this way? What should I be careful of if I'm editing the code? If this appears incomplete, does someone know about it, and where should I go to find out more?

both username and #xxx qualifiers help with those issues. (I don't have strong feelings against FIXME, by the way—that's just now I myself use it).

[smola]

I also sometimes use XXX: to denote a block of code that I envision to be problematic in some edge cases, or to document some non-obvious assumptions that are known to not always be true or, in general, add a word of caution of something that is very hacky in a way that is not obvious to the reader.

On the other hand, I have checked remaining XXX: comments added by me to src-d/go-git and they could be regular comments without any special prefix too...

[juanjux]

I use XXX for things I've to fix before doing the PR because it was easy to grep, but a lot of people use XXX today so I probably should switch to something else :)

[dennwc]

So it looks like the consensus here is in favor of the change, so I'll write a specific proposal for it (PR).