[webkit-dev] review queue crazy idea

Eric Seidel eric at webkit.org
Wed Jul 21 16:47:40 PDT 2010


Patches sit in the queue for numerous reasons.  Some of us used to
scan the queue every day.  But I've stopped doing that.  Now I scan it
more like once a week or two.

There is no good way to find "which patches might I have a chance of
reviewing", so you end up spending 30 minutes just to find a patch you
could actually review.

Most patches get rejected for easily-bot-detectable reasons.  Like bad
or missing ChangeLogs, poor comment style, tabs, breaking EWS bots.
Now that the style bot and EWS bots work better we should at least cq-
patches which fail those bots (or fail to apply).

I think reviewing would be much easier if we had some better site by
which to review.  I'm not sure how we solve the social problem of
getting people to review patches which didn't come from the person
sitting next to them in their office however.  Then again, that's
sorta OK.  part of contributing to webkit is integrating into the
community.  It's importnat to know your reviewers and to discuss
things with them in channels other than the bug.  But I still think
some minimal technical work would go a long way to making reviewing
better.

-eric

On Wed, Jul 21, 2010 at 7:34 PM, Zoltan Horvath <zoltan at webkit.org> wrote:
> Hey,
>
> I just don't understand how can the patches can sit in bugzilla unreviewed for
> weeks? There are 76 reviewers in the trac's team list.
>
> I think the reviewers have to do what they have assumed... Reviewing patches!
>
> I agree with Maciej automatic rejection is unfriendly. (Of course we can
> reject patches which are no longer applies on trunk. Yes, we should do this!)
> I think we should find a good way to advise the reviewers of the patch's topic.
> I prefer this way of automation.
>
> Regards,
>
> Zoltan
>
> On Wednesday 21 July 2010, at 23:40, Ojan Vafai 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. For people
>> new-ish to the WebKit project, it is often confusing both degree of
>> responsibility that lies with the contributor to make the patch easy to
>> review and the need to get reviewers' attention for a given patch.
>>
>> 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. On the other hand,
>> knowing that patches will magically fall off the end of the queue might
>> encourage reviewers to just ignore some patches.
>>
>> An alternative to auto-rejecting patches would be to send a nag email once
>> a week to webkit-reviewers@ with the list of patches that are over a month
>> old.
>>
>>
>> Here are my initial thoughts on what a review bot would do.
>>
>> *After a patch turns a week old, send the following email:*
>> Patch 12345 of bug 6789 is a week old. It may just be because no reviewer
>> has found time to review it. But there may be steps you can take to help
>> get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a
>> few suggestions.
>>
>> -WebKit review bot
>>
>> *After the patch is three weeks old:*
>> Patch 12345 of bug 6789 is three weeks old. If it is still unreviewed in a
>> week, it will automatically be rejected. It may just be because no reviewer
>> has found time to review it. But there may be steps you can take to help
>> get your patch reviewed. See http://trac.webkit.org/wiki/CodeReview for a
>> few suggestions.
>>
>> -WebKit review bot
>>
>> *After the patch is a month old:*
>> Patch 12345 of bug 6789 has been rejected because it is too old. This is
>> likely because no webkit reviewer has been able to review it. If you would
>> still like the patch reviewed, then please do the following:
>>
>>    1. Make sure your patch still applies to tip of tree.
>>    2. Do as many of the suggestions at
>>    http://trac.webkit.org/wiki/CodeReview as possible.
>>    3. Upload your patch for review again.
>>
>> -Webkit Review Bot
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>


More information about the webkit-dev mailing list