97 lines
5.7 KiB
Markdown
97 lines
5.7 KiB
Markdown
## Pull Request Review Policy
|
||
|
||
This document details the pull request policy of the Status Mobile repository / team.
|
||
|
||
- [Terms](#terms)
|
||
- [Policy](#policy)
|
||
- [Overview](#overview)
|
||
- [Full Policy](#fully-policy)
|
||
- [Rationale](#rationale)
|
||
- [Front pressure and Demand control](#front-pressure-and-demand-control)
|
||
- [Back pressure and Supply control](#back-pressure-and-supply-control)
|
||
- [Honour System](#honour-system)
|
||
- [Review Support](#review-support)
|
||
- [Background](#background)
|
||
|
||
### 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:
|
||
|
||
- [2023-05-15 - Council of Deadwater](https://docs.google.com/document/d/1TtUZCke2OzumjdWGu6TIECHEOUPwvaLrvQ8Re2nEGCo)
|
||
- [2023-06-13 - Mauve Council of Deadwater](https://docs.google.com/document/d/19hfr8xXrShLk9ROdQfOwb9_vwbTah9nOCqKSLIrgoSI)
|
||
|
||
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.
|