[webkit-dev] Comments in the code (Was Please include function-level comments in change log entries)
rniwa at webkit.org
Fri Jul 13 11:49:59 PDT 2012
On Fri, Jul 13, 2012 at 11:13 AM, Alec Flett <alecflett at chromium.org> wrote:
> On Fri, Jul 13, 2012 at 10:56 AM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>> On Fri, Jul 13, 2012 at 5:57 AM, Stephen Chenney <schenney at chromium.org>wrote:
>>> I don't doubt there are poor comments, both outdated and useless.
>>> That's a reviewing failure. You have simply highlighted the fact that any
>>> standard for comments requires reviewer attention. Hence "cost of
>>> maintaining comments".
>> I don't know how to review a patch and make sure all relevant comments
>> are updated.
>> As I have illustrated before, you can be modifying a function X, then a
>> completely random function A which calls B that in turn calls C that in
>> turns D ... that in turn calls X may have a comment dependent on the
>> previous behavior of X without ever mentioning X. How am I supposed to know
>> that there is such a comment?
>> But the exact same thing can be said of code. Comments are not special in
> this regard.
I'm not buying that argument because your example is not convincing.
A simplistic example: You could be reviewing code that calls a method that
> takes an int, but in practice that method should never take a negative
> number. (it might even be documented correctly as such)
I would have r-ed the original patch that added this function. We should
either change the argument's type to unsigned or ASSERTed that it's
non-negative. As such, I wouldn't consider this to be neither patch
author's nor reviewer's fault.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the webkit-dev