Skip to content

Commit

Permalink
Update review-procedure.md
Browse files Browse the repository at this point in the history
  • Loading branch information
SlamBamActionman authored Nov 14, 2024
1 parent 968ce46 commit d0f58f9
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions src/en/wizden-staff/maintainer/review-procedure.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Not following this procedure/policy will result in disiplinary action being take
- For merging, this takes the form of 2 "Approval" checkmarks from Maintainers on the PR (visible in the PR's comments or under "Reviewers" in the Github sidebar). This Approval can also be given in written form during discussions on other platforms such as Discord or the SS14 forums, as long as it is also stated by another Maintainer in a comment under the PR. If the PR author is a Maintainer, this automatically counts as the PR having 1 Approval. *An "Approval" checkmark implies that you have reviewed and approved the PR's contents (code, art, mapping change etc.). If you approve of the PR conceptually without reviewing it this can be written in a PR comment, but does not count as an Approval. The label "S: Conceptual Approval" is reserved for Maintainer Discussions and may not be used in this instance.*
- For closing, this takes the form of 2 Maintainers expressing an opinion to close the PR, or a reluctance to implement the PR, in the PR's comments. This "Disapproval" can also be given in written form during discussions on other platforms such as Discord or the SS14 forums, as long as it is also stated by another Maintainer in a comment under the PR.
- 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.
## 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.*
Expand Down

0 comments on commit d0f58f9

Please sign in to comment.