Guidelines for Reviewing Pull Requests
Revision as of 00:05, 3 July 2017 by AndreiAlexandrescu (talk | contribs) (Created page with "Our [http://github.com/dlang github repository] is experiencing a high volume of pull requests (PRs) that are waiting reviews. This document sets forth a few guidelines for re...")
Our github repository is experiencing a high volume of pull requests (PRs) that are waiting reviews. This document sets forth a few guidelines for reviewing pull requests in our community.
Guidelines
- The fundamental dynamics is: both submitter and reviewer want to improve the state of affairs. If the reviewer finds the PR is not entirely adequate, he or she should make suggestions that take the PR from where it is to where it should be.
- Do not ask for overengineering the trivial. It is easy to find small issues with many of the lines in a PR. The response should be proportional to the issue. Do not make one small matter the focus of the entire review.
- The main question about each individual review point is: does this block acceptance or not?
- If the reviewer envisions a more ample solution to a superset of the problem addressed by the PR, and if the PR adequately solves the smaller problem, the reviewer should not burden the submitter with the larger problem. The reviewer should instead accept the PR and then file the larger idea as an enhancement issue, suggest the submitter to work on that next, or work on it straight away.
- Do not hold code hostage on a matter of preference of the reviewer. The submitter and the reviewer are not in opposite camps; instead, they both fight the problem that the PR is attempting to address. In doing so, they are both allied to a common objective standard of quality and professionalism.
- Reviewer should be reasonable in the requests made. Submitter should be liberal in accepting reviewer suggestions.
- No henpecking please. Avoid protracted backs-and-forths on inconsequential matters. Reviewers should make only one pass for minor non-blocking style suggestions, then not insist if they are not followed.
- The reviewer should exercise good judgment instead of forging guidelines and precedents into ironclad rules. (And yes, that applies to the present set of guidelines!)