[webkit-dev] Comments in the code (Was Please include function-level comments in change log entries)

[Ryosuke Niwa]

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.

- Ryosuke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120713/0e3ea77b/attachment.html>