[llvm] r189704 - Remove the suggestion to not duplicate comments in header and

David Blaikie dblaikie at gmail.com
Sun Sep 1 10:33:19 PDT 2013


On Sun, Sep 1, 2013 at 8:49 AM, Chris Lattner <clattner at apple.com> wrote:
> On Aug 31, 2013, at 9:12 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>> > Actually it did:
>> >
>> > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120910/150633.html
>>
>> Ok, it looks like all the discussion was in favor of this never being
>> added to CodingStandards :-)
>
> Not quite sure where you got that idea, though I'll admit the web archive
> isn't the best ui for reading things (but easiest way to reference it,
> searching your own email archive tends to be the more effective way to read
> the whole thread
>
> That thread seems to conclude pretty clearly that removing the copy is a bad
> idea.

As I said, the web archive isn't giving you a complete view of the
thread - it breaks it up by week, which is really unhelpful.

Specifically, I assume you're referring to Andrew's last email on that
week's chunk of the thread (
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120910/150652.html
) arguing by absurdity that removing comments from implementations is
equivalent to removing parameter names from declarations. Chandler
addressed those concerns in
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120910/150657.html
(web archive didn't thread that properly as a reply to Andrew,
strangely) and Andrew agreed to the patch in (
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120917/151191.html
)

In case you don't have the thread in your email (which is way easier
to read) here are all the bits of the thread in the web archive:

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120910/thread.html#150633
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120917/thread.html#151393
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120917/thread.html#151182
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120924/thread.html#151745
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20121001/thread.html#152465
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20121015/thread.html#153895

Searching my email for "r166378" finds the full 19 message, month-long
code review thread as is our usual technique here. I'm not sure it
could've been made any more visible/discoverable.

>
> Andrew conceded to the patch later in the thread and eventually chandler
> signed off on it.
>
>
> That thread also doesn't include that, though I admit that the webarchive
> isn't the easiest to navigate.
>
> This patch was appropriately pre-commit reviewed with discussion and
> disagreement, reverting/undoing it outright seems somewhat poor form.
>
> I'm fine with reverting it while we discuss, I didn't realize it was
> reviewed and approved.  It looked like it was a small paragraph that snuck
> in with a bigger (and great!) enhancement to the docs.

Perhaps the commit comment could've been more descriptive. Dmitry's
reply to the original thread included the revision number so the
pre-commit review is as discoverable as it could hope to be.

> I'll revert it for now.

Thanks - might be worth replying to the original thread to provide
everyone else with context (but those who delete old email would still
be in trouble, since replies don't always include the whole
thread/context (& there are multiple branches, etc)) for further
discussion.

- David



More information about the llvm-commits mailing list