diff --git a/README.mkd b/README.mkd index 9be24a6be35431e44ec38f8d0f226854efb24474..bf11387068e41b85dcc7e77bb0294c24c82ecf94 100644 --- a/README.mkd +++ b/README.mkd @@ -44,7 +44,7 @@ But being a member of the GHC Team carries extra power, as well as extra respons **Responsibilities** * Actively contribute to GHC, through patches, code review, design discussions, infrastructure. "Actively contribute" means at least with a tempo of every month or two. e.g. making a small intervention once a year is not really enough to be a member of the team. * Monitor, and when appropriate contribute to, the [`ghc-devs` mailing list](https://mail.haskell.org/mailman/listinfo/ghc-devs). -* Willingness to proactively review merge requests, when the expertise fits. +* Willingness to proactively review merge requests, when the expertise fits. See the [merge request review policy](./review-policy.mkd) for discussion. * Willingness to undertake some specific obligations, to take responsibility for some specific part of GHC diff --git a/review-policy.mkd b/review-policy.mkd new file mode 100644 index 0000000000000000000000000000000000000000..747eba7cb8a0e595ff17bbec291311f4beeadc80 --- /dev/null +++ b/review-policy.mkd @@ -0,0 +1,83 @@ +# GHC MR review policy + +This document defines the GHC HQ policy regarding merge requests to GHC, in +particular defining responsibilities and expectations for MR authors, reviewers +and the GHC Team. See also +[contributing a patch](https://gitlab.haskell.org/ghc/ghc/-/wikis/Contributing-a-Patch) and +[merge request conventions](https://gitlab.haskell.org/ghc/ghc/-/wikis/gitlab/merge-requests) +which cover the practical implementation of this policy. + +The GHC Team is the core group that develops and maintains GHC (see [the ghc-hq +README](https://gitlab.haskell.org/ghc/ghc-hq/-/blob/main/README.mkd) for +background). Membership of the GHC Team is open to everyone actively +contributing to GHC. Of course, many GHC contributors are not members of the GHC +Team, but they are still strongly encouraged to review MRs where possible. + + +## 1. Responsibilities of MR Authors + +The authors of merge requests are the part of the life-blood of GHC. Our goal +is that their efforts should be focused on the MRs themselves; we do not want +them to get discouraged by a lack of reviews, or inability to land a MR. + +At the same time, reviewing is painstaking work, also done by volunteers, and +authors cannot expect a blank cheque on reviewers time. + +MRs often go through an extended period of drafts, sharing with specific +colleagues, using CI to discover problems. But at some point an author may +think + * This MR is done; it is ready to land; or + * I have a good draft, but I need guidance before investing more time. + +At this point, an author can add a "Blocked on Review" label, as a specific +request to the GHC Team for feedback. + +Landing MRs that make significant changes has a deliberately high bar, because +changes can potentially lead to significant problems later (e.g. outright bugs, +code quality issues, performance regressions). Authors of MRs cannot necessarily +expect others to do work for them: if an MR does not meet these high standards, +the author themselves needs to take responsibility for improving it until it is +ready to land. + +Some patches are difficult to review due to their complexity or due to the +limited number of individuals with knowledge of the code in question, so authors +may need to be patient and accept that reviews take time. + + +## 2. Responsibilities of users assigned to MRs + +Gitlab allows MRs to be assigned to an individual or group. The assignee will be +responsible for moving the MR forward in some way. + +Individuals assigned to perform review (whether in the GHC Team or not) should +either do so promptly, or if they are not in a position to review (e.g. due to +lack of time or necessary expertise) reassign the MR appropriately. Being +assigned as a reviewer does not compel the individual to submit a review, but +they should feel an obligation not to be a bottleneck. + +“Moving the MR forward in some way†includes requesting changes or giving the +author feedback that the MR is unlikely to be accepted, then assigning it back +to the author. Reviewers are not necessarily expected to finish off MRs that are +not yet in a state to merge. + + +## 3. Responsibilities of the GHC Team + +The GHC Team is collectively responsible for: + +* Establishing and communicating standards for MR quality (e.g. keeping the wiki + up to date with what is required of MRs, and pointing out where certain + improvements are needed for an MR to be mergeable). +* Establishing processes (e.g. GitLab labels) for keeping track of MRs in + different stages. +* Identifying MRs that are in need of review and assigning them to appropriate + individual reviewers (inside or outside the GHC Team). +* Encouraging reviewers to review MRs promptly and helping reassign MRs where a + reviewer is not able to do so. +* Ensuring that all MRs are reviewed in a reasonable time frame. +* Helping resolve disagreements about whether a particular MR is acceptable. +* Accepting and landing MRs once they have passed review. + +Members of the GHC Team will not be expected to review every single MR among +themselves. Rather we expect them to solicit and rely on reviews from other +trusted members of the community.