<span class="Apple-style-span" style="font-family: sans-serif; font-size: 13px; border-collapse: collapse; ">It makes no sense to me to r- a patch because reviewers don&#39;t have time to review it. It put incentive in the wrong place. There are other solutions to this problem that put incentive in the right place (i.e. with reviewers). I don&#39;t necessarily think these are good ideas, but I&#39;ll throw them out there.<div>

<br></div><div>1. If the review queue gets too large, close the tree to commits until the queue is at 80% of that too large number.</div><div>2. Automate landing commits (e.g. click a button off the commit queue) so that it&#39;s easier to land commits and keep the commit queue small.</div>

<div>3. Give some kind of very visible notice on the waterfall when the review queue gets too large (e.g. make the background color of the whole page red).</div><div>4. Auto-assign reviews that haven&#39;t gotten reviewed after two weeks to a reviewer and have that person be responsible for reviewing it in a timely manner (this gives personal responsibility over the queue length instead of the current distributed responsibility).</div>

<div><br></div><div>I&#39;m sure there are a ton of other possible solutions to this problem that don&#39;t punish people sending patches who have little control over this situation. I imagine doing 3 and 4 would make a significant difference.</div>

<div><br></div><div>Ojan</div></span><br><div class="gmail_quote">On Fri, May 22, 2009 at 1:52 PM, Eric Seidel <span dir="ltr">&lt;<a href="mailto:eric@webkit.org">eric@webkit.org</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

Sorry I missed on on IRC, I would have been happy to chat further.<br>
<br>
<br>
Our current defacto policy requires involvement on both sides.<br>
Submitters need to be involved in finding people to review their<br>
patches.  Posting patches to the review queue does not automatically<br>
get you a review, except occasionally by Darin Adler or myself.<br>
<br>
If a bug totally stalls, and is sitting in the review queue untouched<br>
I view that as the responsible reviewers&#39; implicit rejection of the<br>
patch.  I, as a responsible reviewer, am simply making explicit that<br>
implicit rejection.  Personally, I&#39;d rather get an r- on my patches<br>
than have them sit ignored for multiple weeks at a time.<br>
<br>
-eric<br>
<br>
<a href="http://webkit.org/coding/contributing.html" target="_blank">http://webkit.org/coding/contributing.html</a>:<br>
Patch review<br>
Once you have a patch file, it must be reviewed by one of the approved<br>
WebKit reviewers. To request a review, attach the patch to the bug<br>
report, and mark the patch with the flag review:?. The reviewer will<br>
typically either approve the patch (by responding with an r=me in the<br>
bug report or in e-mail and marking the patch review:+) or request<br>
revisions to the patch (and mark the patch review:-). In rare cases a<br>
patch may be permanently rejected, meaning that the reviewer believes<br>
the feature should never be committed to the tree. The review process<br>
can consist of multiple iterations between you and the reviewer as<br>
revisions are made to your patch.<br>
<div><div></div><div class="h5"><br>
On Fri, May 22, 2009 at 1:22 PM, Maciej Stachowiak &lt;<a href="mailto:mjs@apple.com">mjs@apple.com</a>&gt; wrote:<br>
&gt;<br>
&gt; On May 21, 2009, at 7:57 PM, Eric Seidel wrote:<br>
&gt;<br>
&gt;&gt; I think it&#39;s better to get things out of the queue then to leave them rot.<br>
&gt;<br>
&gt; But it&#39;s not the patch submitter&#39;s fault if the reviewers have been<br>
&gt; delinquent in review. And making them resubmit the same patch after a<br>
&gt; blanket r- is useless busywork. Per our policy you shouldn&#39;t mark a patch r-<br>
&gt; unless you are either requesting revisions or rejecting the whole concept of<br>
&gt; the patch &lt;<a href="http://webkit.org/coding/contributing.html" target="_blank">http://webkit.org/coding/contributing.html</a>&gt;. So if you r-&#39;d any<br>
&gt; patches without giving review comments, please restore them.<br>
&gt;<br>
&gt; Regards,<br>
&gt; Maciej<br>
&gt;<br>
&gt;<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; Your review are most welcome. :)<br>
&gt;&gt;<br>
&gt;&gt; -eric<br>
&gt;&gt;<br>
&gt;&gt; On Fri, May 22, 2009 at 12:48 PM, Maciej Stachowiak &lt;<a href="mailto:mjs@apple.com">mjs@apple.com</a>&gt; wrote:<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; On May 21, 2009, at 7:27 PM, Eric Seidel wrote:<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt; Our review process seems to be failing.  As a reviewer, let me extend<br>
&gt;&gt;&gt;&gt; my apologies to the WebKit community as I am part of this failure.<br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt; We have over 100 patches in the review queue at the moment:<br>
&gt;&gt;&gt;&gt; <a href="http://webkit.org/pending-review" target="_blank">http://webkit.org/pending-review</a><br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt; I&#39;ve started going through the list and reviewing what patches I can.<br>
&gt;&gt;&gt;&gt; I&#39;m also marking r- all patches I can&#39;t review which have had no<br>
&gt;&gt;&gt;&gt; comments in the last 2 weeks.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; While it&#39;s great to get more reviews done (and I encourage other<br>
&gt;&gt;&gt; reviewers<br>
&gt;&gt;&gt; to get cracking as well), I don&#39;t think it&#39;s appropriate to r- patches<br>
&gt;&gt;&gt; that<br>
&gt;&gt;&gt; you don&#39;t know how to review, simply because no one else has reviewed<br>
&gt;&gt;&gt; them<br>
&gt;&gt;&gt; yet. It is better to have an honest backlog than to sweep things under<br>
&gt;&gt;&gt; the<br>
&gt;&gt;&gt; carpet.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; Regards,<br>
&gt;&gt;&gt; Maciej<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;<br>
&gt;<br>
&gt;<br>
_______________________________________________<br>
webkit-dev mailing list<br>
<a href="mailto:webkit-dev@lists.webkit.org">webkit-dev@lists.webkit.org</a><br>
<a href="http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev" target="_blank">http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev</a><br>
</div></div></blockquote></div><br>