User:Phalacee/Peer Review

Peer review check-list

These are the things I have picked up from peer-reviewing issues. They can also be applied when solving issues. If you can think of anything that needs to be added, please do so. (Thanks Sam for your suggestions)

Syntax

Ensure that:

• there are no unnecessary blank lines in the new code;
• blank lines do not contain spaces;
• variables are named correctly (all lower case, no underscores);
• functions are named correctly; and
• there are no unnecessary changes to whitespace in other areas on the file.
• new strings Don't Use Camel Case

See the Coding style guide for details.

Output

Ensure that:

• the code doesn't use buffered output unless absolutely necessary; and
• lang-strings are used instead of hard-coded strings for text output.

Databases

If there is SQL code you can test quickly, do so.

Ensure that:

• there are minimal DB calls (no excessive use of the DB); and
• the code uses SQL compatible with all the supported DB engines.

Misc

Ensure that:

• the code seems to solve the described problem;
• there are specific testing instructions that state how, as well as what, to test;
• the code doesn't use deprecated functions (see Deprecated functions in 2.0);
• the code makes sense in the broader scheme of things (look at the whole function, not just the altered code); and
• the code is easy to understand, and where it isn't, comments have been provided.
• It doesn't break XHTML
• It doesn't remove strings, or rename strings from stable branches (permitted only in master)
• php docs have been updated and adhere to coding style guide
• The idea/solution makes sense and is complete.
• The developer has searched for and fixed other areas that may also have been affected (provided they are in the scope of the original issue)
• String changes include AMOS syntax where required (moving, copying, deleting strings)
• No inline styles have been used in HTML output (everything has to be in CSS)
• CSS has been added to the appropriate CSS files (base, specific area, sometimes canvas)
• $_GET, $_POST, $_REQUEST, $_COOKIE, and $_SESSION are never used
• Unit tests all pass for related areas where changes have been made
• Uses sesskey for all write actions where appropriate (some read actions as well although not as common)

GIT

• The commit message includes the tracker issue number
• Git history is clean
• Make sure the original author of the work has been given credit within the commit (as author of in the commit message if changes were made)

See Also

• Code checker plugin