[webkit-dev] review queue crazy idea

[James Robinson]

I've had patches sit in the review queue for >4 weeks then receive a
positive review and land without much incident.  Some patches are difficult
to review or have a limited number of potential reviewers.  I would have
really appreciated a reminder email about that patch in particular (I
honestly had forgotten about it by the time the review email came) but it
would not have been helpful to mark it review-.  I would have just marked it
review? again, which given the eventual outcome (r+ with some comments)
would have been the right move.

We should make it clearer to new contributors that it is the role of the
patch submitter to sell it to the project.  This means making it clear why
the patch is an improvement, making it easy for the reviewer to understand,
and making a convincing argument that the patch is correct by thorough
testing.  This also tends to mean figuring out who could review a patch and
chasing them down via email or IRC.  I think if contributors thought in
these terms then it would naturally encourage the behaviors reviewers like -
breaking patches up, providing more test cases, and clearly explaining what
a patch is trying to do.

- James


On Wed, Jul 21, 2010 at 10:17 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai <ojan at chromium.org> wrote:
>
>> There are currently 38 (of 171 total) patches in the review queue where
>> the bugs have not been modified in over 1 month old. I propose we have a bot
>> that educates people about writing easy to review patches and auto-rejects
>> any patches in bugs that haven't been touched in over a month.
>
>
> I'd love to see a bot that educates people but I'm not sure if we should
> auto-rejects patches that are simply old. Maybe we need a little bit more
> of graduality like pinging the author of the patch first, and then make it
> obsolete if nobody responds etc...
>
> This is just an initial proposal. I'm not wed to any of the details of how
>> this would work. I do think that auto-rejecting old patches is valuable to
>> the project as a whole. Having the review queue be so large makes
>> it daunting for any reviewer to try and tackle it.
>>
>
> I'd agree that having a really large review queue isn't ideal.  Maybe we
> can customize the review queue so that it only shows patches of your
> expertise.
>
> Best,
> Ryosuke Niwa
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100721/c24f685d/attachment.html>