On Fri, Jul 13, 2012 at 11:13 AM, Alec Flett <alecfl...@chromium.org> wrote:
> On Fri, Jul 13, 2012 at 10:56 AM, Ryosuke Niwa <rn...@webkit.org> wrote: > >> On Fri, Jul 13, 2012 at 5:57 AM, Stephen Chenney >> <schen...@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
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev