Define pr-review.md practice #24
Description
With maintainer defined, we should define some standard practices regarding PR review.
Brain-dumping here, but some things I would consider useful to define are:
- Expectation on review frequency (e.g. each dev should block a 1 period of time for code review every day, but not be expected to review code more than once per day)
- Expectation on clarity of reviews (e.g. A review must specify Approval, Requested changes, or recommend closing the PR. NIT/NB comments alone does not constitute a review)
- Typical PR practices (e.g. every PR must have min. 1 approving review)
etc.?
This is likely to be VERY closely related to as-of-yet undefined GitHub practices.
See also #3
Original context
I think there's adequate implication here for what happens if there's a disagreement with a PR from someone else, e.g. "Maintainers are the decision-makers for structural changes or contentious features within the repository.", however, we currently have an implicit "all PRs should be approved by at least one other person", which leaves a significant gap for PRs that the maintainer themselves create, e.g. if a maintainer's PR is contentious for some reason and no reviewer has/will approve it and/or if reviewer's have not reviewed the PR for some amount of time, then maintainer's responsibility becomes impossible and there's nothing that they can do about it.
Originally posted by @cjyetman in #12 (comment)