[webkit-dev] Why I'm reviewing patches outside my area (and why you should too)

[Jeremy Orlow]

On Tue, Mar 9, 2010 at 9:39 PM, David Levin <levin at chromium.org> wrote:

>
>>  (3) Someone reviewed an earlier version of the patch but then didn't
>> follow up.  I think having a partial review by one person encourages
>> other people to assume that person will finish the review.  That cause
>> the patch to float upwards in pending-review until it gets lost in the
>> sea of ancient patches.  I'd encourage reviewers to follow through
>> with their reviews
>
>  ....
>
>> Don't be afraid of r-ing a patch.  Believe me,
>> folks are thankful for feedback (even negative feedback) when their
>> patches have been sitting around collecting dust.
>>
>
> However as soon as you r- a patch, according to "3", you need to finish the
> review when it gets re-submitted. This leads me to believe that *one
> shouldn't avoid a patch that has feedback from someone else* unless one
> doesn't feel qualified to judge whether the feedback has been addressed.
>
> I plea to folks submitting patches:
> 1. Keep your patches as small as possible. It is no fun to deal with a 60K
> patch.
> 2. Review your own patches before or right after you attach them to the bug
> as if you were reviewing someone else's code.
> 3. Address any style issues, build issues that the bots notice. You don't
> need to wait for a review to point out the same issue.
>

It's also a big help when peers (which aren't necessarily WebKit reviewers)
look over it and give review-style feedback as well.  Especially when said
peers know more about that code than any of the official reviewers.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100309/d7d3a73f/attachment.html>