[webkit-dev] RenderArena: Teaching an old dog new tricks

[Geoffrey Garen]

On Nov 14, 2012, at 3:27 PM, Ojan Vafai <ojan at chromium.org> wrote:

> As someone outside all these discussions, this seems like a completely unfair characterization of what happened. Sam voiced an objection, then there was a bunch of discussion in which Chris made an argument that Eric found compelling. Many days passed, then Eric r+'ed. Unless there was other discussion not on the bug that I missed, your objection came after Eric's r+. I don't see the process problem here.

I want to call this issue out specifically, because it has come up more than once now.

The burden of proof for a patch is on the submitter, not on the reviewer. If a reviewer says a patch is a bad idea, it's up to the submitter to convince the reviewer otherwise. Saying more things and then letting time pass is not the same as convincing the reviewer, and it does not give one automatic rights to commit code, just because the reviewer has not kept up with line-by-line refutation.

It is an impossible burden for good reviewers to argue, line-by-line, against all bad ideas. I tried to do this in just one bug (<http://webkit.org/b/88834>), saying no to O(N^2) algorithms, followed by leaky algorithms, followed by use-after-free algorithms, with specific details, and the full process took three months. It is simply not practical to assume that reviewers can do this for all patches.

During those three months, one of the reasons I kept up with line-by-line refutation was that I feared that another reviewer might use the same logic you've used here in order to r+ a bad patch. This has to stop.

The process problem here was three-fold:

(1) Chris argued with Sam's objection, but did not resolve it

(2) Eric r+ed a patch with an unresolved objection from a reviewer

(3) Eric announced a new direction for WebKit, after two reviewers had objected to it

On Nov 14, 2012, at 3:18 PM, Adam Barth <abarth at webkit.org> wrote:

> One thing that I notice about these two comments is that they say
> "please don't make this change" but they don't explain why.  I realize
> that it takes more time and energy to explain the "why," but it's
> helpful for folks like me (and I suspect Chris and Cris as well) who
> don't know these issues as well as you.
> 
> Thanks for taking the time to explain the issue in your previous
> email.  That helped me understand this issue much better than before.

I'm happy to participate, and say "why", when I have time. I don't always have time.

Please don't take my participation as consent to the crazy notion that line-by-line refutation is required to r- a bad patch.

Geoff

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20121115/af9b5172/attachment-0001.html>