Switch from TODO(owner) to TODO(issue) going forward?

[Gregg Tavares]

Chromium currently has the requirement that a TODO comment left in the code needs to be in form TODO(owner): msg or TODO: owner - msg

WDTY about changing the requirement to TODO(issue): msg and TODO: issue - msg ?

// TODO(12345678): Fix the thing

// TODO(crbug.com/12345678): Fix the thing

// TODO(https://crbug.com/12345678): Fix the thing

// TODO:12345678 - Fix the thing

// TODO:crbug.com/12345678 - Fix the thing

// TODO:https://crbug.com/12345678 - Fix the thing

Would all be valid

Owners leave the team so having TODO(person-no-longer-available) is not that useful. Issues stick around and allow discussion and context.

The actual check would just just switch from checking TODO(anything): and TODO:anything -  to TODO(anything\d+): and TODO:anything\d+ -  

Thoughts?

[Joe Mason]

Chrome already allows TODO(bug number). In fact it's preferred: see https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++-dos-and-donts.md#comment-style.

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

[Gregg Tavares]

> Chrome already allows TODO(bug number). In fact it's preferred:

Then my suggest is to 

(1) change the presubmit check so it requires an bug number and fails on just an owner

(2) change the message so it says "TODO missing bug number" instead of what it says now which is "TODO missing owner"

Can we do this or will their be pushback?

[Joe Mason]

+chromium-dev, which is a better place for this discussion about coding style.

I think it'd be too restrictive to hard require a bug number. Changes to the presubmit that would actually block submissions need a lot of thought and testing, because there are so many devs that could be affected. But it should be uncontroversial to change the error message on a bare TODO to "missing crbug number". The message should nudge people toward the more preferred format.

Maybe it could give an error on TODO and a non-blocking warning on TODO(owner). I don't know if it's worth the complexity, though.

[K. Moon]

FWIW, I've found TODO(person) to generally be useless. Either they're TODOs to myself because I was too lazy to file a bug, or they're for someone else and I ignore them. The intent of using them to ask someone about a TODO they wrote is rare, in my experience.

If we want to get ambitious, one clever bit of automation: Rather than a blocking or submit, automatically replace non-conforming TODOs with a new bug. That lowers the friction, but I'm not sure I'd love the spamminess.

(Maybe AI can take a first stab at finding a relevant bug. 🙃)