Skip to content

Latest commit

 

History

History
96 lines (71 loc) · 5.65 KB

pr-review-policy.md

File metadata and controls

96 lines (71 loc) · 5.65 KB
Raw

Pull Request Review Policy

This document details the pull request policy of the Status Mobile repository / team.

Terms:

Term Definition
Requester The team member submitting a pull request.
Reviewer A team member asked by a requester to review the requester’s pull request.
Review bandwidth The number of reviews a team member has self-limited themselves to giving within their defined review period.
Review period The period of time over which a reviewer’s bandwidth is consumed

Policy:

Overview

  • 🔀 - You are responsible for getting your PR reviewed and approved.
  • ✅ - You must give a review on a PR if you are asked to do so, unless your review limit has been reached.
  • ✨ - The honour system underpins this policy, without acting in good faith it will not work.

Fully policy

  1. The requester has the responsibility for getting a PR reviewed and approved.
    1. The requester has the right and responsibility to pursue code reviews.
    2. The requester has the right and responsibility to make multiple requests of the same reviewer.
    3. The requester will select a limited number of reviewers to review any of the requester’s PRs.
  2. Reviewers are obligated to give a review for all PRs they are requested to review, except when their review bandwidth has been exhausted:
    1. Reviewers have the right to refuse a requester a review if the reviewer’s review bandwidth has been exhausted.
    2. Reviewers are expected to manage and track their review bandwidth.
    3. Reviewers can manage their review bandwidth via any means they feel works best for them. Examples include:
      1. Using the GitHub busy feature
      2. Declining a review on the PR
      3. Declining a review when pursued by a code reviewer
      4. Proactively declining to review a PR
  3. All team members will set a review bandwidth that is reasonable.
  4. All team members acknowledge that giving reviews is a requirement of membership of the team.
  5. All team members will manage their review bandwidth honestly.
    1. The right to refuse a PR review is only eligible if a reviewer’s review bandwidth has actually been exhausted.
    2. Breaching the team’s honour is very bad form. Do not do it (please).
  6. All team members acknowledge that without acting with honesty this review process will not work and jeopardises their own future ability to receive reviews.

Rationale

Front pressure and Demand control

Foremost in the rationale for the above policy is that giving a PR review is a requirement. If a requester requests a PR review of a team member, the reviewer is now obligated to review the PR within a reasonable timeframe.

This principle enforces that there is a pressure and demand that PR reviews must be given.

Back pressure and Supply control

Front pressure alone is not enough to balance the PR review load across the team. Because of this an exception is granted to reviewers that have exhausted their review bandwidth, the exemption permits these reviewers to refuse a PR review.

This principle gives the reviewer pool back-pressure, relieving the exhausted reviewer and increasing pressure / demand on the other reviewers.

Honour system

The two principles of demand and supply control are underpinned by an honour system and are useless without honest action. If a team member does not honestly manage their review bandwidth this action seriously damages the collective balance of PR review load. Because of this potential damage a breach of the honour system is considered a deeply shameful act.

The honour system is the mechanism of control and enforcement, without it the policy would need to be controlled and enforced using less dynamic and forgiving mechanisms. An example is a GitHub review counting tools with quotas and metrics. So just be cool.

Review Support

Sometimes reviewing is time-consuming or generally painful, to help make reviews as simple as possible consider the following suggestions:

  • Paired reviews: Ask an experienced reviewer to review with you.
  • If a PR is the product of multiple developers only ask 1 or 2 other developers for their review.

Background

The contents of this policy document is the product of 2 councils of the Status Mobile team, the below documents summarise the topics and outcomes of the councils:

The aims of the council were to allow developers that are relied on heavily for reviews to better balance their time between reviews and other responsibilities. Equally, the council aimed to give other team members better opportunity and understanding that giving PR reviews is a requirement and will help the team generally if more review burden is shared.