From 230f5425ad07fb70316ebd33e750dd6f1e33c42b Mon Sep 17 00:00:00 2001 From: SlamBamActionman <83650252+SlamBamActionman@users.noreply.github.com> Date: Thu, 26 Dec 2024 13:30:33 +0100 Subject: [PATCH 1/2] Added exemptions and cleaned up the text a bit --- .../wizden-staff/maintainer/review-procedure.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/en/wizden-staff/maintainer/review-procedure.md b/src/en/wizden-staff/maintainer/review-procedure.md index e1cc49b18..97eadc655 100644 --- a/src/en/wizden-staff/maintainer/review-procedure.md +++ b/src/en/wizden-staff/maintainer/review-procedure.md @@ -1,6 +1,7 @@ # PR Review Procedure -This does not apply to rule change or server config change PRs from the Head Game Admins. In an emergency these requirements may be waived with written permission from a Project Manager (either on Github, Discord or the SS14 forums). -Not following this procedure/policy will result in disiplinary action being taken. + +This documents lists the Maintainer procedure for reviewing and merging PRs to the [Space Station 14 repository](https://github.com/space-wizards/space-station-14). + ## Requirements - All PRs **should** adhere to the [code conventions](../../general-development/codebase-info/conventions.md), but this is *not* a hard requirement since some specific usecases may require straying from convention for readability. - PRs **should** be triaged, as per the [triage procedure](triage-procedure.md) guidelines. If a PR is not triaged, this can be done at the same time as the PR is initially considered for review. @@ -11,6 +12,14 @@ Not following this procedure/policy will result in disiplinary action being take - In the case where there's dissent among maintainers, e.g. 2 Approvals and 1 Disapproval, a Maintainer Discussion should be held as described under the Policy section. - You do not need to be one of the two Maintainers who have given Approval/Disapproval to merge/close the PR. Ideally a PR should be processed once it meets the requirements to be merged/closed, but if that does not happen for some reason, any other Maintainer is free to do it (even if that Maintainer is the PR author). - PRs that have more than 3 weeks of inactivity waiting on something from the PR author should be considered "stale" and labeled as such with the `S: Stale` label, unless another reason explains the inactivity (e.g. freeze or author has informed about a longer break). You may use any means to contact the author of the PR, but at the minimum you should leave a comment on the PR and give at least 1 week to respond before closing a stale PR. A PR that is waiting on an action from Maintainers (e.g. a code review or discussion resolution) is not eligible to be marked as stale. + +### Exemptions: +- PMs and Wizards may waive any PR Review Procedure requirements if deemed necessary for the health of the project. When doing so, it should be done via written permission with an explanation (either on Github, Discord or the SS14 forums). Note that this exemption is not the default for actions taken by the relevant roles, and should be explicitly invoked when used. +- Maintainer Maptainers (i.e. Maintainers with additional responsibilities regarding Mapping) may ignore the "2 Maintainer" requirements for merging/closing any PRs involve mapping changes. This means a only single Maintainer Maptainer is needed to approve and merge mapping PRs, and also are able to self-merge mapping PRs. + - Map tooling PRs are not included in this exemption. + - Map hotfix PRs are not included in this exemption and must still follow normal hotfix procedure. +- Rule change PRs and server config change PRs submitted by (or with written approval from) a Head Game Admin do not need to adhere to the PR Review Procedure. + ## Policy - When starting a new review for a PR that has not yet been assigned to a Maintainer, you are **required** to Assign yourself to the PR. This can be done under "Assignees" in the Github sidebar. This let others know that you will be "owning" the PR. If you decide to stop owning the PR, un-assign yourself so that others may take over the PR. By owning a PR you are taking responsibility for keeping track of the PR's progress, ensuring code quality and keeping the PR author informed of what is the status of the review process. If you are the PR author yourself, you may not also be the PR owner. Anyone may review an owned PR. *If changes are Approved/Disapproved by the PR owner any Maintainer may merge the PR if they also Approve/Disapprove the changes, provided there is no other Maintainer dissent.* @@ -31,6 +40,8 @@ Not following this procedure/policy will result in disiplinary action being take - Any PR that is made for illegitimate reasons, such as abuse, spam or harrassment, may be closed without following the Policy as long as a PM has given written permission, as per the procedures set out in the Requirements section. +- Not following the procedure/policy outlined in this document will result in disiplinary action being taken, at the discretion of SS14's Wizard team. + ## Issues & Reviews While Issues do not have any code to review, they may still contain suggestions and feature proposals that would benefit from a Maintainer Discussion. From 19dae0f54a88a86d32dc8099be5309d2191cd4cb Mon Sep 17 00:00:00 2001 From: SlamBamActionman <83650252+SlamBamActionman@users.noreply.github.com> Date: Thu, 26 Dec 2024 13:31:22 +0100 Subject: [PATCH 2/2] Update review-procedure.md --- src/en/wizden-staff/maintainer/review-procedure.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/en/wizden-staff/maintainer/review-procedure.md b/src/en/wizden-staff/maintainer/review-procedure.md index 97eadc655..5793092f0 100644 --- a/src/en/wizden-staff/maintainer/review-procedure.md +++ b/src/en/wizden-staff/maintainer/review-procedure.md @@ -40,7 +40,7 @@ This documents lists the Maintainer procedure for reviewing and merging PRs to t - Any PR that is made for illegitimate reasons, such as abuse, spam or harrassment, may be closed without following the Policy as long as a PM has given written permission, as per the procedures set out in the Requirements section. -- Not following the procedure/policy outlined in this document will result in disiplinary action being taken, at the discretion of SS14's Wizard team. +- Not following the procedure/policy outlined in this document will result in disiplinary action being taken, at the discretion of the SS14 Wizard team. ## Issues & Reviews While Issues do not have any code to review, they may still contain suggestions and feature proposals that would benefit from a Maintainer Discussion.