On Nov 14, 2012, at 3:27 PM, Ojan Vafai <o...@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 <aba...@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

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to