Difference between revisions of "Code Review"
| Line 1: | Line 1: | ||
| − | + | ||
| + | <div class="warnbox"><b>Warning</b>: The following information is obsolete, and may quite possibly be incorrect. Obsolete articles are candidates for deletion. Information posted to official communities should be assumed to be accurate. If you have fresh information, please update this article (and remove this textbox!)<br><br><font size="-1"><center>This box was added on {{CURRENTMONTHNAME}} {{CURRENTDAY}}, {{CURRENTYEAR}}.</center></font></div>[[Category: Obsolete]] | ||
| + | |||
| + | {{Note|text=This page has not been updated for the git conversion.}} | ||
Code Review is an important task that must be done for patches submitted to [[Bugzilla]]. By doing this task, you help <dwuser>mark</dwuser> and <dwuser>fu</dwuser> spend less time doing code review and more time coding. | Code Review is an important task that must be done for patches submitted to [[Bugzilla]]. By doing this task, you help <dwuser>mark</dwuser> and <dwuser>fu</dwuser> spend less time doing code review and more time coding. | ||
Revision as of 12:00, 16 March 2013
Code Review is an important task that must be done for patches submitted to Bugzilla. By doing this task, you help ![[info]](http://wiki.dreamwidth.net/wiki/extensions/LJ/images//dw/user.png){kind=small}
mark and fu spend less time doing code review and more time coding.
Here are bugs in need of review.
Contents
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.
Build a test-case
Look at what the patch is supposed to fix and reproduce it on your own installation. Already at this step it is best to think of several ways this patch can be tested so you can see what happens before and after you apply the patch.
Apply the Patch
Update to the latest committed code, as per the instructions in Dev Maintenance. Apply the patch from the pull request. 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.
Suggestions for things to test for, as applicable: test with paid, free, personal and community accounts, logged in and logged out, use different site schemes and layouts, try foreign characters in user input or empty input, entries and user names that don't exist, and anything else you can think of.
Make appropriate changes to 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 turn the review tag to review+.
- If you find a patch still needs work, you can explain in a clear and friendly manner what should be done, and turn the review tag to review-.
- 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!)
