[llvm] r213197 - trivial fix for PR20314

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed Jul 16 16:02:21 PDT 2014


> On 2014-Jul-16, at 15:30, Sanjay Patel <spatel at rotateright.com> wrote:
> 
> I'll checkin with fixes again shortly, but I'm curious about this one:
> "We don't usually put bug numbers in the comments.  Please remove."

IMO, the two biggest problems with PRs in comments are: they detract
from the readability of the code itself, and they get stale as the code
changes.  The latter defeats their benefit (and compounds the former).

I've just checked the coding standards [1], and I don't actually see a
rule there about listing PRs in comments.  Nevertheless, it's not usually
done (likely for the same reasons I don't like them).

[1]: http://llvm.org/docs/CodingStandards.html

> This fix is too easy and I agree, but for larger bug fixes that I've made over the last few weeks, I've referenced bug numbers in comments. I think that's for the best for whoever wanders into that code blindly next.

Not sure what you mean here -- if it's understanding the code, then the
comments should be sufficient (without PRs); if it's finding the history,
then `git log` and `git blame` (or `svn log/blame`) do the job more
accurately and consistently.

> I'm purposely being very verbose in bugzilla so others can clearly see how the code came to be.

I think verbosity in bugzilla is great.  Quoting the PR in the commit
message (which you did) allows future developers to look up the bug and
see your comments.

> I'm new to this codebase, and I'm cursing the lack of comments and external references for big blocks of logic that I'm trying to debug.

As mentioned above, you can use `git blame` for the external references,
but lack of comments around complicated blocks of logic is bad.  Ideally,
we'd all fix the ones that we took the time to understand :).



More information about the llvm-commits mailing list