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

David Levin levin at chromium.org
Tue Mar 9 13:39:51 PST 2010


>
>
>  (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.

dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100309/df00dfb0/attachment.html>


More information about the webkit-dev mailing list