From 12233a58ef5f54bfceeb3e153ae004d19eb39b74 Mon Sep 17 00:00:00 2001 From: Princess Cheeseballs <66055347+Princess-Cheeseballs@users.noreply.github.com> Date: Sat, 23 Aug 2025 19:47:50 -0700 Subject: [PATCH 1/6] Update maintainer-policy.md Borgar --- src/en/wizden-staff/maintainer/maintainer-policy.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/en/wizden-staff/maintainer/maintainer-policy.md b/src/en/wizden-staff/maintainer/maintainer-policy.md index fe6fabcf1..2db0a78f4 100644 --- a/src/en/wizden-staff/maintainer/maintainer-policy.md +++ b/src/en/wizden-staff/maintainer/maintainer-policy.md @@ -11,6 +11,18 @@ This policy applies in addition to the [General Staff](../staff-policy.md) and [ - Maintainers should try to perform code reviews *whenever possible*, getting content into the game is everyone's responsibility. - If there is a conflict around game design between something in the docs and someone's idea, what is detailed in the documentation takes precedent. If a change to specific documentation is needed, it should be brought up with the respective work group for discussion and changes should be made to the documentation if needed. +## Deferral Policy +- Maintainers are not expected to know everything about every part of the game, but when reviews are made they are expected to at least have oversight from someone who does know. +- Sometimes a PR is a blocker, or critical to the game in some way where regular review procedure needs to be expidited, in a way different from p0 issues and hotfixes +- In these cases if a Maintainer is unable to complete a review themselves they should defer to a maintainer who can. +- Deferrals are different from normal reviews in that it is a whole review split between two or more maintainers. + - In a deferral a maintainer completes their review but defers part of the review to a maintainer with more expertise than them. + - This would be effectively asking another maintainer to review one specific file/system/concept being modified. + - The maintainer being defered to does not need to do a full review with approval, but just needs to approve the code relevant to their expertise +- Deferrals should be asked for in a separate channel from normal reviews such that they don't get buried +- Deferrals should only be used if a PR is important, this includes: bugfixes, blockers, and otherwise nonspecified important content +- If a majority of a PR requires a deferral, you may still use deferral channels to ask another maintainer to review, but it may be better that said deferred maintainer does a full review. + ## In-game Permissions - Maintainers can request to be given in game permissions by the admin team. - Maintainers may only use the permissions they are given for the purposes of debugging issues, and playtesting new features. From 2e814ccc8c2bfade4c58494a286d5dc0672e5994 Mon Sep 17 00:00:00 2001 From: Princess Cheeseballs <66055347+Princess-Cheeseballs@users.noreply.github.com> Date: Sun, 24 Aug 2025 00:57:26 -0700 Subject: [PATCH 2/6] Clarification spelling mistakes and better wording --- .../maintainer/maintainer-policy.md | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/en/wizden-staff/maintainer/maintainer-policy.md b/src/en/wizden-staff/maintainer/maintainer-policy.md index 2db0a78f4..f7f62b052 100644 --- a/src/en/wizden-staff/maintainer/maintainer-policy.md +++ b/src/en/wizden-staff/maintainer/maintainer-policy.md @@ -13,15 +13,23 @@ This policy applies in addition to the [General Staff](../staff-policy.md) and [ ## Deferral Policy - Maintainers are not expected to know everything about every part of the game, but when reviews are made they are expected to at least have oversight from someone who does know. -- Sometimes a PR is a blocker, or critical to the game in some way where regular review procedure needs to be expidited, in a way different from p0 issues and hotfixes -- In these cases if a Maintainer is unable to complete a review themselves they should defer to a maintainer who can. -- Deferrals are different from normal reviews in that it is a whole review split between two or more maintainers. - - In a deferral a maintainer completes their review but defers part of the review to a maintainer with more expertise than them. - - This would be effectively asking another maintainer to review one specific file/system/concept being modified. - - The maintainer being defered to does not need to do a full review with approval, but just needs to approve the code relevant to their expertise +- In cases where a PR has higher need to be merged than others but an active maintainer cannot fully complete a PR review on their own they may defer a part of their review to another maintainer. + - Case can include but are not limited to Blocking PRs, Bugfixes, and Important Community Content PRs. +- Deferrals are different from normal reviews in that it is a full review split between two or more maintainers. + - When a maintainer defers their review, they complete as much of the review as possible and then hand over the sections of the PR they need help with to a more knowledgeable maintainer. + - The maintainer who is deferring the review should specifiy what part of the code they are deferring and request a maintainer review it for them they should be specific about what or who they need if possible. + - The maintainer should also specify why they are deferring it rather than requesting a review + - Since deferring is meant to speed up review procedure this is typically for PRs that are important to merge and should clarify that importance. + - Deferrals should be used to hasten the review process without sacrificing code quality, so a maintainer deferring part of their review should be able to complete most of the review themselves. - Deferrals should be asked for in a separate channel from normal reviews such that they don't get buried -- Deferrals should only be used if a PR is important, this includes: bugfixes, blockers, and otherwise nonspecified important content -- If a majority of a PR requires a deferral, you may still use deferral channels to ask another maintainer to review, but it may be better that said deferred maintainer does a full review. + - This channel should also be used for important PRs that cannot be deferred due to being mostly or fully requiring the expertise of another maintainer +- When a maintainer has a PR deferred to them, they should follow normal review procedure with some changes: + - A maintainer doing a deferred review doesn't have to sign off on the entire PR, just the section sent to them + - A maintainer can request changes and block the PR from being merged as normal + - When a maintainer approves of the section assigned to them, they should express so in a comment on github and notify the maintainer who deferred the review to them. + - A maintainer is not stopped in any way from doing a full review with a full approval themselves. +- Once all deferred parts of the review are approved can the original maintainer finish their review and approve the PR + - Note that this only counts as approval of the original maintainers, any maintainers who reviewed a deferred part of the PR can still review the PR later and approve it themselves. ## In-game Permissions - Maintainers can request to be given in game permissions by the admin team. From 8c142c319639c8f1cf1f9ab1f081f5cc4973faf4 Mon Sep 17 00:00:00 2001 From: Princess Cheeseballs <66055347+Princess-Cheeseballs@users.noreply.github.com> Date: Mon, 6 Oct 2025 18:07:57 -0700 Subject: [PATCH 3/6] remove from maint policy --- .../maintainer/maintainer-policy.md | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/en/wizden-staff/maintainer/maintainer-policy.md b/src/en/wizden-staff/maintainer/maintainer-policy.md index f7f62b052..fe6fabcf1 100644 --- a/src/en/wizden-staff/maintainer/maintainer-policy.md +++ b/src/en/wizden-staff/maintainer/maintainer-policy.md @@ -11,26 +11,6 @@ This policy applies in addition to the [General Staff](../staff-policy.md) and [ - Maintainers should try to perform code reviews *whenever possible*, getting content into the game is everyone's responsibility. - If there is a conflict around game design between something in the docs and someone's idea, what is detailed in the documentation takes precedent. If a change to specific documentation is needed, it should be brought up with the respective work group for discussion and changes should be made to the documentation if needed. -## Deferral Policy -- Maintainers are not expected to know everything about every part of the game, but when reviews are made they are expected to at least have oversight from someone who does know. -- In cases where a PR has higher need to be merged than others but an active maintainer cannot fully complete a PR review on their own they may defer a part of their review to another maintainer. - - Case can include but are not limited to Blocking PRs, Bugfixes, and Important Community Content PRs. -- Deferrals are different from normal reviews in that it is a full review split between two or more maintainers. - - When a maintainer defers their review, they complete as much of the review as possible and then hand over the sections of the PR they need help with to a more knowledgeable maintainer. - - The maintainer who is deferring the review should specifiy what part of the code they are deferring and request a maintainer review it for them they should be specific about what or who they need if possible. - - The maintainer should also specify why they are deferring it rather than requesting a review - - Since deferring is meant to speed up review procedure this is typically for PRs that are important to merge and should clarify that importance. - - Deferrals should be used to hasten the review process without sacrificing code quality, so a maintainer deferring part of their review should be able to complete most of the review themselves. -- Deferrals should be asked for in a separate channel from normal reviews such that they don't get buried - - This channel should also be used for important PRs that cannot be deferred due to being mostly or fully requiring the expertise of another maintainer -- When a maintainer has a PR deferred to them, they should follow normal review procedure with some changes: - - A maintainer doing a deferred review doesn't have to sign off on the entire PR, just the section sent to them - - A maintainer can request changes and block the PR from being merged as normal - - When a maintainer approves of the section assigned to them, they should express so in a comment on github and notify the maintainer who deferred the review to them. - - A maintainer is not stopped in any way from doing a full review with a full approval themselves. -- Once all deferred parts of the review are approved can the original maintainer finish their review and approve the PR - - Note that this only counts as approval of the original maintainers, any maintainers who reviewed a deferred part of the PR can still review the PR later and approve it themselves. - ## In-game Permissions - Maintainers can request to be given in game permissions by the admin team. - Maintainers may only use the permissions they are given for the purposes of debugging issues, and playtesting new features. From 35e1b43d6d2104dc2e61a2cc6f64253ff944eff1 Mon Sep 17 00:00:00 2001 From: Princess Cheeseballs <66055347+Princess-Cheeseballs@users.noreply.github.com> Date: Mon, 6 Oct 2025 18:17:07 -0700 Subject: [PATCH 4/6] Update review-procedure.md Move policy to review procedure. --- src/en/wizden-staff/maintainer/review-procedure.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/en/wizden-staff/maintainer/review-procedure.md b/src/en/wizden-staff/maintainer/review-procedure.md index 9a8b524b1..1b1b63ec3 100644 --- a/src/en/wizden-staff/maintainer/review-procedure.md +++ b/src/en/wizden-staff/maintainer/review-procedure.md @@ -29,6 +29,8 @@ This documents lists the Maintainer procedure for reviewing and merging PRs to t ## 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.* +- Maintainers are not expected to know everything about every part of the game, but when reviews are made they are expected to at least have oversight from someone who does know. If you are reviewing a PR but do not have the confidence to fully complete the review due to a lack of expertise with a system, you should contact another maintainer with experience to help with the review. When helping another maintainer with a review, you can do a deferred review instead of a full review if you want. When doing a deferred review, you don't have to review the entire PR, but you should clearly state that this is a deferred review so that other Maitainers know that you don't plan on doing a full review on the PR. Doing a deferred review does not stop you from doing a full review later, but it also does not require you to explictly approve of the PR, since it is not a full review. + - If you think a PR warrants further discussion with maintainers, or there is dissent regarding merging/closing, you may **optionally** start a Maintainer Discussion. **This is not a requirement**, but is recommended when you aren't sure about something. If you choose to do this, you are **required** to tag the PR with the "S: Under Maintainer Discussion" label, which should automatically cause a thread with the PR's name, PR number, Github link and appropriate tags/pings to be created on Discord in the #maint-review channel. If this does not happen automatically (e.g. the Discord bot is down or otherwise unavailable), you should create the thread manually. Any maintainer may do this, not just the PR owner but make sure to check to not make a duplicate. Once discussion has concluded, summarize the result of the discussion in a PR comment *before* taking any action. Then, remove the `S: Under Maintainer Discussion` label. Closing or merging should automatically apply appropriate tags and title prefixes to the discussion thread (e.g. [Approved], [Merged], [Closed] etc.), as well as close it. - If the result of a Maintainer Discussion is positive and the PR has not undergone code review (meaning it is not yet ready to merge), the PR may be given the label `S: Conceptual Approval`, to show that it has been discussed with a positive outcome. - If the result of a Maintainer Discussion is negative (i.e. a desire to close the PR), the result of the discussion can be cited as the reason to close the PR without explicitly requiring 2 Disapprovals. From 074b9815564abc486b47e299733896788695f103 Mon Sep 17 00:00:00 2001 From: Princess Cheeseballs <66055347+Princess-Cheeseballs@users.noreply.github.com> Date: Mon, 6 Oct 2025 18:20:44 -0700 Subject: [PATCH 5/6] merg conflict --- src/en/wizden-staff/maintainer/review-procedure.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/en/wizden-staff/maintainer/review-procedure.md b/src/en/wizden-staff/maintainer/review-procedure.md index 1b1b63ec3..9a8b524b1 100644 --- a/src/en/wizden-staff/maintainer/review-procedure.md +++ b/src/en/wizden-staff/maintainer/review-procedure.md @@ -29,8 +29,6 @@ This documents lists the Maintainer procedure for reviewing and merging PRs to t ## 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.* -- Maintainers are not expected to know everything about every part of the game, but when reviews are made they are expected to at least have oversight from someone who does know. If you are reviewing a PR but do not have the confidence to fully complete the review due to a lack of expertise with a system, you should contact another maintainer with experience to help with the review. When helping another maintainer with a review, you can do a deferred review instead of a full review if you want. When doing a deferred review, you don't have to review the entire PR, but you should clearly state that this is a deferred review so that other Maitainers know that you don't plan on doing a full review on the PR. Doing a deferred review does not stop you from doing a full review later, but it also does not require you to explictly approve of the PR, since it is not a full review. - - If you think a PR warrants further discussion with maintainers, or there is dissent regarding merging/closing, you may **optionally** start a Maintainer Discussion. **This is not a requirement**, but is recommended when you aren't sure about something. If you choose to do this, you are **required** to tag the PR with the "S: Under Maintainer Discussion" label, which should automatically cause a thread with the PR's name, PR number, Github link and appropriate tags/pings to be created on Discord in the #maint-review channel. If this does not happen automatically (e.g. the Discord bot is down or otherwise unavailable), you should create the thread manually. Any maintainer may do this, not just the PR owner but make sure to check to not make a duplicate. Once discussion has concluded, summarize the result of the discussion in a PR comment *before* taking any action. Then, remove the `S: Under Maintainer Discussion` label. Closing or merging should automatically apply appropriate tags and title prefixes to the discussion thread (e.g. [Approved], [Merged], [Closed] etc.), as well as close it. - If the result of a Maintainer Discussion is positive and the PR has not undergone code review (meaning it is not yet ready to merge), the PR may be given the label `S: Conceptual Approval`, to show that it has been discussed with a positive outcome. - If the result of a Maintainer Discussion is negative (i.e. a desire to close the PR), the result of the discussion can be cited as the reason to close the PR without explicitly requiring 2 Disapprovals. From f3a648248a83775c9d75181a80e255aff4d1b38d Mon Sep 17 00:00:00 2001 From: Princess Cheeseballs <66055347+Princess-Cheeseballs@users.noreply.github.com> Date: Mon, 6 Oct 2025 18:34:03 -0700 Subject: [PATCH 6/6] Update review-procedure.md --- .../maintainer/review-procedure.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/en/wizden-staff/maintainer/review-procedure.md b/src/en/wizden-staff/maintainer/review-procedure.md index 51399b01e..572e8d7b6 100644 --- a/src/en/wizden-staff/maintainer/review-procedure.md +++ b/src/en/wizden-staff/maintainer/review-procedure.md @@ -223,3 +223,27 @@ People can often be emotional after their PR is closed. - When requesting changes to a PR be made, keep them within the scope of the PR. For example, while requesting small numerical adjustments on a `No C#` PR is fine, it would exceed the scope of the PR to request a new system to be added. In cases like these, it is best to close the PR and explain the changes that would need to be made. + + +- If a PR touches code outside of your area of experise and you are not confident in properly reviewing it, you should try and contact another maintainer to help with the review if possible. + + +- If you are helping another maintainer review a PR but do not want to do dedicate yourself to a full review, you should try and follow partial review guidelines. + + +## Partial Review guidelines. + +- Maintainers are not expected to be familiar with every system in the game or have knowledge of all fields of expertise in computer science. As such it is always okay to ask another maintainer to help with your review. + +- This policy exists to lower the workload on maintainers who are requested to help with a review due to their expertise as well as improve communication around partial reviews. A maintainer may choose to leave a full review and ignore these guidelines if they want. + +- If you leave a partial review you should clearly state so. You should also try to state why the review is partial for example: + - There were a number of standout coding issues that should be addressed before leaving a full review. + - This PR touches an area of your expertise and you noticed some coding issues you need addressed, you do not plan to leave a full review. + - You wanted to request a couple general things you want changed, but do not plan on returning to the PR later. + +- Explaining why you're leaving a partial review helps communicate your intentions to other maintainers so that they can do full reviews later, and approve once their reviews are finished and your changes have been addressed. + +- If another maintainer does not state their intention to come back to a PR later after leaving a partial review, it's best to assume they won't be coming back to do a full review. + +- As usual, doing a partial review never exempts you from changing your mind and doing a full review later. Partial reviews are purely for helping with a PR's quality without having to dedicate yourself to a full review.