Peer reviewing
From MoodleDocs
Peer review check-list
These are points to consider while peer-reviewing issues. They can also be applied when solving issues, to reduce the likelihood of problems during peer-review and integration.
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)
Tracker
- Appropriate labels are added if needed (Reference :- Tracker Issue Labels)
- If the fix introduce a need to alter/add something to dev docs, it must be accomplished before the issue is closed.
GIT
- The commit message includes the tracker issue number and the component. Ideal format is
MDL-xxxx Component Commit message
- 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)