Review Policy

Peer review is a proven method to improve code quality and is therefore strongly encouraged.

1. As the Contributor

1. 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).
2. 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.
3. Discuss objections. Do not override a -1 unless there is a broad expert consensus that the objection is unfounded.
4. Do not ignore/fight the Early Warning System. Justify each override.
5. 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
     1. at least one review (+1) from somebody else is present, and
     2. nobody else who could approve (+2) the change can be produced within reasonable time.

2. As the Reviewer

1. Ensure compliance with the Commit Policy.
2. 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.
3. 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.
4. If you are not really interested in a change, say so and/or give a +1 at most.