OK, let's keep it then. I changed the comment in question to be
"TODO(KUDU-1537)...".

On Thu, Sep 15, 2016 at 8:18 PM, Jake Farrell <jfarr...@apache.org> wrote:
> we use "TODO(bug id or committer id): msg" as the format in other projects
> and that seems to be enough breadcrumb in most cases
>
> -Jake
>
> On Thu, Sep 15, 2016 at 11:06 PM, Todd Lipcon <t...@cloudera.com> wrote:
>
>> On Thu, Sep 15, 2016 at 6:50 PM, Adar Dembo <a...@cloudera.com> wrote:
>>
>> > In https://gerrit.cloudera.org/#/c/4435/, Tidy Bot said:
>> >
>> > Line 209:       // TODO: Should be fixed with Exactly Once semantics,
>> > see KUDU-1537.
>> > warning: missing username/bug in TODO [google-readability-todo]
>> >       // TODO: Should be fixed with Exactly Once semantics, see
>> KUDU-1537.
>> >       ^
>> >       // TODO(unknown): Should be fixed with Exactly Once semantics,
>> > see KUDU-1537.
>> >
>> > This doesn't look like the kind of style change we want, right?
>> > Historically we don't annotate our TODOs with names.
>> >
>> > Or should I reformat it as "TODO(KUDU-1537)..." ?
>> >
>>
>> Yea, I think TODO(bug#) is a good policy to try to have moving forward, but
>> I don't think we have to be religious about it. I can turn off this tidy
>> check if we think it's not worth it with a codebase of our size.
>>
>> This guideline comes from Google (
>> https://google.github.io/styleguide/cppguide.html#TODO_Comments) where
>> they
>> say:
>>
>> TODOs should include the string TODO in all caps, followed by the name,
>> e-mail address, bug ID, or other identifier of the person or issue with the
>> best context about the problem referenced by the TODO. The main purpose is
>> to have a consistent TODO that can be searched to find out how to get more
>> details upon request. A TODO is not a commitment that the person referenced
>> will fix the problem. Thus when you create a TODO with a name, it is almost
>> always your name that is given.
>> It's sort of nice to leave a breadcrumb, but it's also not too hard for
>> someone to 'git blame' and figure out who added it, so I could go either
>> way.
>>
>> -Todd
>>
>>
>> >
>> >
>> > On Wed, Sep 14, 2016 at 10:04 PM, Todd Lipcon <t...@cloudera.com> wrote:
>> > > Hey folks,
>> > >
>> > > I've set up a jenkins job and gerrit trigger to run clang-tidy-diff on
>> > any
>> > > patches that are uploaded. It should be set up now so as not to vote +1
>> > or
>> > > -1, but just to produce comments. For an example of the type of
>> warnings
>> > it
>> > > generates, check out:
>> > > https://gerrit.cloudera.org/#/c/4409/4/src/kudu/consensus/
>> > raft_consensus_state.h
>> > >
>> > > If you see any checks that you think are false positives, feel free to
>> > ping
>> > > me and I can either disable those checks entirely, or see if there's
>> some
>> > > configuration we can make to better match our own guidelines.
>> > >
>> > > Hopefully this turns out to be a useful bit of "automatic code review"
>> so
>> > > that we can focus our review efforts less on mechanical checks and more
>> > on
>> > > things requiring human judgment :) If it turns out to be more of an
>> > > annoyance than a help, we can always remove it or really dial back to
>> > only
>> > > the most important warnings.
>> > >
>> > > Also worth noting that these checks are not that complicated to write,
>> so
>> > > if we see that there are some Kudu-specific ones worth implementing, we
>> > can
>> > > easily do so.
>> > >
>> > > -Todd
>> > > --
>> > > Todd Lipcon
>> > > Software Engineer, Cloudera
>> >
>>
>>
>>
>> --
>> Todd Lipcon
>> Software Engineer, Cloudera
>>

Reply via email to