Changes between Initial Version and Version 1 of CodeReview


Ignore:
Timestamp:
Jul 2, 2010 11:54:49 AM (14 years ago)
Author:
ojan@chromium.org
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • CodeReview

    v1 v1  
     1Once you put a patch up for review (i.e. mark a patch r?), it goes into the [https://bugs.webkit.org/request.cgi?action=queue&requester=&product=&type=review&requestee=&component=&group=requestee Review Queue]. 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:
     2
     3 * Look at the people who most recently modified the code you're changing and explicitly CC them on the bug.
     4 * 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.
     5 * 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.'''
     6 * 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.
     7 * 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.
     8 * 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.
     9 * Fix errors from any bots that run after you upload your patch.
     10
     11If 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.