[webkit-dev] getting patches reviewed

Ojan Vafai ojan at chromium.org
Fri Jul 2 11:59:42 PDT 2010


I tried to give guidelines for getting a patch reviewed at
http://trac.webkit.org/wiki/CodeReview. Feel free to add to or modify that
list. I've included the contents of that page below:

Once you put a patch up for review (i.e. mark a patch r?), it goes into the
 Review Queue<https://bugs.webkit.org/request.cgi?action=queue&requester=&product=&type=review&requestee=&component=&group=requestee>.
That may not be enough to get it reviewed in a timely manner. WebKit
reviewers are all overloaded. The responsibility is on the committer to make
it as easy as possible to get the code reviewed. To that end, you should:

   - Look at the people who most recently modified the code you're changing
   and explicitly CC them on the bug.
   - Add a layout test when you fix a bug/change a behavior or explain why
   that isn't possible and add a manual test to WebCore/manual-tests.
   - Make your patch as small as possible. *Really, seriously, go out of
   your way to make sure you've broken down your patch into the smallest
   self-contained chunks possible.*
   - Only do/fix one thing per patch. Don't change a behavior and then add a
   fix for an incidental thing that you noticed while doing the original
   change. It should be two patches.
   - Do style cleanup or other significant refactoring in a separate patch,
   preferably as a precursor to the patch you want to land. Especially end of
   line whitespace cleanup as this just adds a lot of visual noise to the
   review.
   - Add short per function comments in the change log (see any of Darin
   Adler's changes for a good example). These help reviewers quickly understand
   why you are doing a change in that function.
   - Fix errors from any bots that run after you upload your patch.

If you've done all the above, then you can also ping on #webkit to see if
there are any reviewers who can review it for you. If it's a trivial change,
than anyone can review it. If it's a complicated or gnarly part of the
codebase, you'll likely need to of the approval of one of the people who
recently modified the code.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100702/f323d4b9/attachment.html>


More information about the webkit-dev mailing list