 | This document is for reviewers. If you would like to become a reviewer, please contact us by writing to sio2@sio2project.mimuw.edu.pl . |
The reviewer sees the change.
Reviewers assigned by the change author are automatically mailed. Please don't automatically ignore these emails, ok? 
If you get a review request, please treat it as a very high priority task. There's another person, who is nervous and has hard time doing something else than waiting for your review.
At the beginning:
- if you see the review request email, but you are busy and you wouldn't quickly do the review, add a comment asking the author to assign someone else; that's fine, it's better if someone else does the review quickly,
Now complete the following reviewer's checklist:
- check that the change description makes sense and is descriptive,
- check that the change description references a Jira issue, if there is one,
- check if you understand the change:
- if you don't understand the change, because you don't have enough knowledge about changed code or used frameworks, just write it in a comment and proceed,
- if you don't understand the change, because it's too complicated for a human to understand, ask the author to simplify it, maybe split into more smaller changes,
- if you think that the change can be made way simpler, way better etc., ask the author to do it,
- if you think that the change can be made a bit simpler, a bit better etc., but needs a substantial rewrite of the change, do not write anything,
- check proper escaping in Jinja templates, if they are touched by the change,
- you don't need to test (run the project) whether the change actually does what it is expected to do, but if you like, you can checkout the code with the change using the command shown by Gerrit. The command does not change the revision your current branch points to, just checks out into an unnamed branch.
- finally, check that the code looks good, that is correctly formatted, obeys coding standards and does not have too long lines (should be marked in Gerrit).
Remember that you can also add comments inline, in the reviewed code, by double-clicking on a line.
| Note Our code need not be the best possible code on the Earth. It must be good enough. |
| Note Gerrit has useful keyboard shortcuts. The most useful one is ?. |
| Note Comments written in Polish are allowed in Gerrit (if the change author speaks Polish of course). We need to be as much productive in code review as possible, and the comments aren't usually used after the change is committed. |
The reviewer admires the change.
A happy reviewer replies with Looks Good To Me status, often abbreviated to LGTM. Do not do "LGTM, but fix method naming before submitting" — this is -1, not LGTM. It is ok to say "LGTM, but you may simplify that, if you like".
 | Warning If the author is in our sio2-developers permission group, it's his/her responsibility to submit the change. Otherwise the reviewer should submit. In both cases, if the change cannot be merged, it's the author who should rebase the change. |
The reviewer does not admire the change.
If the change has at least one problem which prevents it from being submitted to the main repository, the reviewer should add a reply with a negative score. A negative score does not mean that the change is bad, ugly, and its author too.
Avoid writing reviews with no score. Adding a score make the change more prominently visible as "reviewed" on the list of the changes.
Each comment is automatically mailed to the author and all other reviewers.