Skip to content

Commit 5b37684

Browse files
Split Content Review Procudure and Doc Review Procedure. Add types of Maintainers to maintainer.md. Add Documaintainer. (space-wizards#590)
## Description Did some organizing/cleanup of the docs repo to add descriptions for the different types of Maintainers and added Documaintainer as a new type of maintainer. Also moved doc review policy to it's own file rather than it being tacked on to the end of content review procedure. ## Why? Documents do not get the maintenance that they deserve, this has been a persistent problem for years. They're also something that we do not need a full content maintainer to maintain, and are of a specialization different from code review. This is a low hanging fruit for a task we can split off into a specialized new staff role. In addition, the docs repo is extremely important. All staff changes, policy changes, and such MUST go through the docs repo. Therefore the more competent hands we can have maintaining the docs repo, the easier it will be to make important policy changes, add new types of specialized staff which allows us to lift a lot of the burden off of our currently small maintainer team. --------- Co-authored-by: Rem <Remfexxel@pm.me> Co-authored-by: Princess Cheeseballs <66055347+Pronana@users.noreply.github.com>
1 parent b194fc8 commit 5b37684

8 files changed

Lines changed: 60 additions & 31 deletions

File tree

src/SUMMARY.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,8 @@ Staff
364364
- [Maintainer Policy](en/wizden-staff/maintainer/maintainer-policy.md)
365365
- [Workgroup Policy](en/wizden-staff/maintainer/maintainer-workgroup-policy.md)
366366
- [Codeowners Policy](en/wizden-staff/maintainer/codeowners-policy.md)
367-
- [Reviewing Procedure](en/wizden-staff/maintainer/review-procedure.md)
367+
- [Content PR Review Procedure](en/wizden-staff/maintainer/content-review-procedure.md)
368+
- [Docs PR Review Procedure](en/wizden-staff/maintainer/doc-review-procedure.md)
368369
- [Review Whitelist](en/wizden-staff/maintainer/review-whitelist.md)
369370
- [Hotfix Procedure](en/wizden-staff/maintainer/hotfix-procedure.md)
370371
- [Triage Procedure](en/wizden-staff/maintainer/triage-procedure.md)

src/en/general-development/codebase-info/releases.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ What does this mean for you? If you're a fork developer, this means that you now
88
Updates are deployed every 14 days over the course of a weekend (Usually on a Saturday). 2 days prior to update day, "master" will be branched into "staging" and maintainers will perform a review of the upcoming changes. During this period, changes may be made or PRs may be reverted on the staging branch. Release day also coincides with the day of the regular maintainer meeting, half of which will be devoted to looking over the changes prepared for release. The outcome of this meeting determines if the update will be deployed as is, receive changes, or be delayed. The following 2 days after an update is deployed will allow for balance/minor gameplay fixes to be deployed under the normal Hotfix procedure.
99

1010
### Review/PR Procedure
11-
All PRs must be reviewed according to the PR review procedure [documented here](../../wizden-staff/maintainer/review-procedure.md), and may be merged to the development branch "master" at any point. PR's fixing code bugs or critical gameplay issues *may* be merged directly onto the "stable" branch but in that case must additionally follow the Hotfix procedure [documented here](../../wizden-staff/maintainer/hotfix-procedure.md).
11+
All PRs must be reviewed according to the PR review procedure [documented here](../../wizden-staff/maintainer/content-review-procedure.md), and may be merged to the development branch "master" at any point. PR's fixing code bugs or critical gameplay issues *may* be merged directly onto the "stable" branch, but in that case must additionally follow the Hotfix procedure [documented here](../../wizden-staff/maintainer/hotfix-procedure.md).
1212

1313
### What is Hotfixable?
1414
Any issue that directly impacts a player's ability to play the game in a largely negative way and can be considered by most to be a "bug" is eligable to be merged as a Hotfix. Critical gameplay issues may also fall into this category as well. "Critical" being an issue that is majorly disruptive to players **or admins**. *Outside of an emergency*, all **Hotfixes require 3 maintainers to sign-off** on merging (Ideally they should also review but giving approval is enough). Bugfixes may be applied to master following the regular review requirements.

src/en/wizden-staff/maintainer.md

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,31 @@
11
# Maintainer
22

3-
This section contains information on Maintainer and PR-Review Policies, **as it pertains to Wizards Den servers, hosted by Space Wizards.** These policies do not apply to other servers, and may be waived in exceptional circumstances.
3+
This section contains information on Maintainer and PR-Review Policies, **as it pertains to Wizards Den servers, hosted by Space Wizards.** These policies do not apply to other servers and may be waived in exceptional circumstances.
4+
5+
## Maintainer
6+
7+
A maintainer is a member of staff who is given authority and trust to review and merge PRs. All maintainers, regardless of specialization, are expected to follow the Maintainer Policy.
8+
9+
### SS14 Maintainer
10+
11+
An SS14 maintainer maintains the content side of Space Station 14. This includes the docs repo and the content repo.
12+
13+
### Documaintainer
14+
15+
A documaintainer is an SS14 maintainer who only has maintainer powers for the docs repository. Within the docs repository, they have the same power and authority as an SS14 maintainer.
16+
17+
### Head Mapper
18+
19+
A head mapper is an SS14 Maintainer who has exclusive power over content PRs with map changes and additions. These maintainers are expected to ensure mapping standards are met, and their approval is required for any PRs that make mapping changes.
20+
21+
### Art Lead
22+
23+
An art lead is an SS14 maintainer who has exclusive power over content PRs with sprite changes and additions. These maintainers are expected to ensure the game maintains its art style, and their approval is required for any PRs that make significant changes to existing sprites or add new sprites.
24+
25+
### UI Lead
26+
27+
A UI lead is an SS14 maintainer who has exclusive power over content PRs with UI changes and additions. These maintainers are expected to ensure the game retains usable, aesthetically pleasing UIs which fit the game's art style.
28+
29+
### Robust Toolbox Maintainer
30+
31+
A Robust Toolbox maintainer is a maintainer for the game's engine, Robust Toolbox. These are the only maintainers who have merge powers for the engine.

src/en/wizden-staff/maintainer/adding-new-maintainers.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Once a new Maintainer has been accepted, they must be on-boarded. The pre-requis
6161
### Additional Notes
6262

6363
- Read the [maintainer tools](/en/wizden-staff/maintainer/maintainer-tools.html) page to see some useful tools that can help you as a maintainer.
64-
- Read the [maintainer policy](/en/wizden-staff/maintainer/maintainer-policy.html) and [review procedure](/en/wizden-staff/maintainer/review-procedure.html) to see which rules you have to follow when merging PRs.
64+
- Read the [maintainer policy](/en/wizden-staff/maintainer/maintainer-policy.html) and [review procedure](/en/wizden-staff/maintainer/content-review-procedure.html) to see which rules you have to follow when merging PRs.
6565
- Read the [doc page](/en/general-development/codebase-info/releases.html) on how our release model and maintainer meetings work. They happen every two weeks and the event schedule can be found on discord.
6666
- After being invited to the GitHub organization, go to the space-wizards org > People > Find yourself > and set your organization visibility to public. This allows people to more easily see you are a maintainer (you will have a “Member” attached to your comments) AND you get to flex the org on your github profile.
6767
- It is recommended to set up two-factor authentication for your Discord, SS14, and github accounts to ensure they cannot be compromised.

src/en/wizden-staff/maintainer/review-procedure.md renamed to src/en/wizden-staff/maintainer/content-review-procedure.md

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# PR Review Procedure
1+
# Content PR Review Procedure
22

33
## Abstract
44
This document outlines the Maintainer procedure for reviewing and merging PRs to the [Space Station 14 repository](https://github.com/space-wizards/space-station-14) `master` branch (otherwise known as the Content repository).
@@ -166,27 +166,3 @@ Next:
166166
If the pull request gets kicked out of the merge queue due to failing tests, check if the test failure is related to the PR. If not, re-add the pull request to the merge queue.
167167

168168
If the test is a Heisentest (a bug causing a rare, sporadic test fail), it should be logged in the [Heisentest bug tracker](https://github.com/space-wizards/space-station-14/issues/41081).
169-
170-
## Reviewing Docs Pull Requests
171-
The Docs repo has a different review policy compared to the Content repository.
172-
Since the Docs repo can be pushed to directly by maintainers, is mostly text-based, can be easily reverted/changed, and does not affect forks, most docs pull requests can be handled by a single maintainer.
173-
174-
### Pushing to the Docs Repo
175-
Maintainers have direct push access to the Docs repo and are encouraged to use it to update any outdated information, whether it's technical documentation, amending a design document, or just fixing grammar.
176-
177-
### PR Reviews
178-
PRs that update instructional or reference documentation (including but not limited to setup guides, style guides, system documentation) require only a single approval before it can be merged.
179-
Likewise, only a single maintainer needs to express concern to close it.
180-
181-
Note that this does not apply to changes to internal procedure or other modifications that require voting or group deliberation according to relevant policy.
182-
183-
#### Design Documents
184-
Major PRs that introduce or change a design document should require at least two maintainer approvals.
185-
If the addition is large enough, a maintainer can call a vote on the design document; however, this should be done only when the scope of the pull request warrants it.
186-
187-
Major game features usually follow the below dynamic:
188-
1. A contributor creates a preliminary design document demonstrating what they would like to add, offering a high-level overview of how it fits into the game.
189-
2. Maintainers discuss the document in an informal discussion. If most agree, the document is marked with `S: Doc Approved`, and the contributor can start work on the actual feature. As the feature is developed, the document is updated if necessary.
190-
3. Once the content-side implementation is ready to be merged, both the design document and the content pull request are merged in tandem.
191-
192-
The intention behind this is to ensure that contributors do not waste time implementing a feature that may not fit well into Upstream's intended gameplay.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
## Docs PR Review Procedure
2+
3+
The Docs repo has a different review policy compared to the Content repository.
4+
Since the Docs repo can be pushed to directly by maintainers, is mostly text-based, can be easily reverted/changed, and does not affect forks, most docs pull requests can be handled by a single maintainer.
5+
6+
### Pushing to the Docs Repo
7+
Maintainers have direct push access to the Docs repo and are encouraged to use it to update any outdated information, whether it's technical documentation, amending a design document, or just fixing grammar.
8+
9+
### PR Reviews
10+
PRs that update instructional or reference documentation (including, but not limited to, setup guides, style guides, and system documentation) require only a single approval before they can be merged.
11+
Likewise, only a single maintainer needs to express concern to close it.
12+
13+
Note that this does not apply to changes to internal procedure or other modifications that require voting or group deliberation according to relevant policy.
14+
15+
#### Design Documents
16+
Major PRs that introduce or change a design document should require at least two maintainer approvals. Unlike other doc PRs, design docs can only be approved by those with content maintainer permissions.
17+
If the addition is large enough, a maintainer can call a vote on the design document; however, this should be done only when the scope of the pull request warrants it.
18+
19+
Major game features usually follow the following dynamic:
20+
1. A contributor creates a preliminary design document demonstrating what they would like to add, offering a high-level overview of how it fits into the game.
21+
2. Maintainers discuss the document in an informal discussion. If most agree, the document is marked with `S: Doc Approved`, and the contributor can start work on the actual feature. As the feature is developed, the document is updated if necessary.
22+
3. Once the content-side implementation is ready to be merged, both the design document and the content pull request are merged in tandem.
23+
24+
The intention is to ensure that contributors do not waste time implementing a feature that may not fit well with Upstream's intended gameplay.

src/en/wizden-staff/maintainer/hotfix-procedure.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Not following this procedure/policy will result in disciplinary action being tak
44
## Requirements
55
- **Three Maintainers must *sign-off*** (Approval is required, reviewing is recommended but optional) on a hotfix PR for it to be merged.
66
- The Hotfix procedure only applies to PRs being merged straight to "stable" or "staging". **When merging bugfixes to master, this procedure does NOT apply, use the normal PR procedure instead!**
7-
- All Hotfixes must adhere to the normal [PR Review Procedure](../maintainer/review-procedure.md) in addition to any requirements listed here.
7+
- All Hotfixes must adhere to the normal [PR review procedure](../maintainer/content-review-procedure.md) in addition to any requirements listed here.
88
- Hotfixes must be given the "Hotfix" label during triage. They will also automatically receive a label indicating their branch.
99
- Stable Hotfixes are for fixing bugs only, not for adding new content or minor balance adjustments. *If a balance issue is bad enough to majorly impact game quality, it should be considered a bug and is eligible for a hotfix. This is up to Maintainer judgment, but if you are unsure, it's recommended to create a discussion thread prefixed with "HOTFIX-PRNumber".
1010
- Hotfixes to Staging may include any changes that are deemed necessary for the upcoming release, but are otherwise still referred to as Hotfixes and follow these procedures.

src/en/wizden-staff/maintainer/maintainer-policy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ This policy applies in addition to the [General Staff](../staff-policy.md) and [
55

66
**Breaking any of these rules will result in disiplinary action being taken**
77
## Reviews and Feedback
8-
- All maintainers must follow the [PR Review](../maintainer/review-procedure.md) and [Hotfix Procedures](../maintainer/hotfix-procedure.md) when they apply.
8+
- All maintainers must follow the [PR review](../maintainer/content-review-procedure.md) and [hotfix procedures](../maintainer/hotfix-procedure.md) when they apply.
99
- Maintainers should try to the best of their ability, to keep PR authors informed on the status of their PRs.
1010
- Maintainers should keep public criticism of code *constructive* and avoid making comments in regards to the authors of the code. **Harsh but fair** citicism of code is *acceptable*, but criticism of its author is not.
1111
- Maintainers should try to perform code reviews *whenever possible*, getting content into the game is everyone's responsibility.

0 commit comments

Comments
 (0)