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. 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) And the only thing you see in the diff is a call that might pass in a negative number due to the way *it* is called. As a reviewer you do your due diligence but still incorrectly assume that negative numbers are fine, perhaps because of the way a function is named. (PositionAtRelativeZLevel or somesuch, which seemed perfectly self documenting at the time, because N years ago RelativeZLevels were never negative, by definition!) Excess comments are far less likely to cause bugs than well-written code. Or put another way, you can write excellent code and still have bugs just as you can write excellent comments and still have typos or they get outdated. Yes, you do your due diligence as a reviewer, but there are limits obviously - the code is written by humans who make mistakes. Reviews drastically limit those mistakes, but so do comments. I think the existence of clang demonstrates that syntax cant catch all pure correctness issues - if anything the reviewer is a higher-level compiler, who happens to also speak english. By rejecting comments, its as though you have so much faith in the compiler and the reviewer-as-language-interpreter, that one could be tempted to use it as a crutch. And yes while incorrect behavior can be observed through automated testing, automated testing does not catch all incorrect behavior, especially unexpected never-before-seen behavior. Why do you think people write fuzzers? This is yet another way that folks are arguing "comments can be wrong, code can't [thanks to compilers and automated testing], so don't write excess comments" Alec > - Ryosuke > > > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev