From Qt Wiki
Peer review is a proven method to improve code quality and is therefore strongly encouraged.
As the Contributor
- Invite relevant reviewers.
- Always invite the respective domain experts, not somebody convenient.
- If everything else fails, broadcast a review request to a relevant mailing list (as usual, make sure to give the mail a meaningful subject).
- Give reviewers ample time to respond.
- Unless everyone who can be reasonably expected to have a relevant opinion to offer has already done so, a full working day waiting time is the absolute minimum; three days are reasonable.
- In particular, give watchers (usually higher-level maintainers) enough time to voice concerns even if you did not explicitly invite them.
- This applies to every new Patch Set in turn.
- Discuss objections. Do not override a -1 unless there is a broad expert consensus that the objection is unfounded.
- Do not ignore/fight the Early Warning System. Justify each override.
- Do not approve your own changes.
- If there is no candidate for a review yet, introduce somebody to the code.
- Maintainer privilege: A maintainer may approve their own change to the code they maintain if
- at least one review (+1) from somebody else is present, and
- nobody else who could approve (+2) the change can be produced within reasonable time.
As the Reviewer
- Ensure compliance with the Commit Policy.
- Make sure the target branch complies with the Branch Guidelines. In case of a mismatch, ask the user to re-target the affected changes server-side.
- Never approve changes outside your expertise.
- While the Approver privilege is granted globally, you are expected to know your limits.
- If you are not certain that you know all relevant policies of a particular sub-project, it's a good sign that you should not approve.
- If you can approve only parts of the change (horizontal, e.g., build system or documentation, or vertical, e.g., only QtCore), state what you approve and give +1 for that. The most appropriate maintainer will apply a vague 1+1=2 rule for the overall approval.
- Do not approve just because it would be convenient for your colleague across the room/corridor.
- If you are not really interested in a change, say so and/or give a +1 at most.