Difference between revisions of "Code Review"

Revision as of 10:40, 3 April 2009

Note: This article is a work in progress and should be reviewed by mark before being taken as advice.

Code Review is an important task that must be done for patches submitted to Bugzilla. By doing this task, you help mark spend less time doing code review and more time coding.

Code Review Steps

Style compliance

Run through the Programming Guideline Checklist. If the patch violates any of these guidelines, reject with specific instructions on what is wrong.

Apply the Patch

Update to the latest committed code, as per the instructions in Dev Maintenance. Apply the patch (you might want to use Mercurial Queues for this--see Dev Version Control). If the patch doesn't apply cleanly, reject with a note to check their patch against the latest committed code. They may have forgotten to update before submitting, or there may have been changes committed since they submitted!

Test the functionality

Note what the bug patch is meant to fix. Verify that the issue is fixed or that the new feature is working properly. Try to see if you can make things break.

Remove the review tag

If the patch follows style guidelines, applies cleanly, and functions, you may now note that you have reviewed the patch for all of these and remove the review tag. If a commit tag is there, leave it. If a commit tag is not there, tell the person that they should add it if they are ready for the patch to be committed. (Sometimes people want patches to be reviewed before they are finished working on them!)