[webkit-dev] Why I'm reviewing patches outside my area (and why you should too)
jorlow at chromium.org
Tue Mar 9 13:51:18 PST 2010
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
> 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...
More information about the webkit-dev