On Tue, Mar 9, 2010 at 9:39 PM, David Levin <[email protected]> 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.
_______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
