Maintainer Guidelines

# Maintainer Guidelines ## Rules for Maintainers #### Repository rules - You may NOT self-merge your own pull requests. * Pull requests made by maintainers should be subject to the same level of scrutiny as that of any other contributor and must be reviewed by a different maintainer. - You may NOT merge any pull request until *at least* 24 hours have passed, and it passes CI completely. * This gives other maintainers and contributors a chance to actually look at the PR and provide their critiques. * There may be exceptions like a critical fix to something, consult with a Headcoder if you think it may be necessary (but it almost never is). - You may not merge a PR that is outside of your specialty(s). * This means that a Maptainer should not be merging code PRs without first getting approval from a Maintainer who specializes in code, and vice-versa. If you'd like to expand upon your specializations speak with a headcoder about it. - **Important**: DO NOT SQUASH TG batch PRs. For TG batch PR's we do merge commits (not squash). It's important that we don't squash them, to preserve the commit history. Merge commits are normally not enabled for the repo so they have to be temporarily allowed by someone with permissions. #### Reviewing PRs - You should: remain calm in the face of criticism, toxicity, or animosity. * Remember that as a member of the Maintainers team you are representing NovaSector. Please refrain from getting into heated arguments or acting unprofessional in Discord. This also applies to your PR reviews. If you feel the heat coming on step away for a few minutes, or whatever you need to do, to keep it classy. - Try to **avoid bikeshedding** (aka, focusing on the trivial). * Style adherence is important, but for smaller things that are unspecified in [STYLE.md](https://github.com/tgstation/tgstation/blob/master/.github/guides/STYLE.md) or [Reviewing a PR](https://hackmd.io/rT1rAIa5Rra6WkifaYWGDg) it really does not matter as much, and almost always isn't worth annoying contributors over. * For example: I don't ever want to see people debating the best number of carriage returns after a proc, or `if()` vs `if ()`. As long as it adheres to TG standards and isn't something wildly disruptive then the personal stylistic choices really don't matter, as long as it remains consistent throughout the file. #### Things that need to be cleared with a headcoder: Maintainers have autonomy in most things however there are a few instances where you should consult with a headcoder first: - Test merges. * unless it is something urgent like an important fix or staff event. - Any PR that you merge which includes a config update. * let a headcoder know right away so that they can update that on the server. Otherwise you will probably forget. - Any PR that modifies save data/persistence (character preferences for example). - Merging a TG Batch PR * once they are ready they will need to turn on merge commits so it can be merged. ## General Expectations - As a maintainer your committment first and foremost is to maintaining the code quality, much of your time spent coding will likely be fixing and reviewing _other peoples_' code (be it PR's from TG or reviewing ones on Nova). Hopefully that is something you enjoy doing! - Everyone should strive to review at least one PR per week, and/or import a batch of PRs from TG. * If you are going to be absent for an extended period (vacation, family emergency, whatever) just let everyone know. - If you are doing a batch of TG PRs, please communicate that with the rest of the team so no one else starts doing it! * Try to push your branch with merge conflicts resolved to GitHub as soon as possible, so as not to hold things up. If you don't have a lot of time to spare you can always do a smaller batch! ## Rank Structure **Headcoder** - This is the top of the rank tree for all intents and purposes, Headcoder can override pretty much any decision made by lower ranks and block any PR. They can merge or close any PR at will. They have the final say regarding disputes. **Maintainer** - Maintainers can override any decisions made by an assistant maintainer provided that they have good reason to. Maintainers can block any PR that is not made by a higher rank provided that they have a reason to. Maintainers may merge or close any PR on GitHub provided it *is in their speciality*. * **IMPORTANT: A maintainer is still **not** considered staff despite having access to staff tools. You should generally not be replying to tickets in-game unless you also have the staff role, or if they are bug-related and you are willing and able to assist.** **Assistant Maintainer** - Assistant Maintainers may not block or close any PR without approval from a Maintainer or Headcoder. They may merge any PR mirrored from TG Station provided they are in a mergeable state. They may moderate development channels and GitHub, however, like Maintainers they are not considered staff and do not have any ability to act as staff when it comes to admin tickets and such. **Inactive Maintainer** - These are maintainers who are not currently active, they do not have any power or permissions over anything. If you are marked as inactive, you must request to be made active again. ## Permissions Chart | Rank | GitHub | In-Game | Discord | | ---------------------- | -------------------------------------------------- | -------------------------- | ----------------------------------------------------------------- | | Headcoder | Everything | Everything | Everything | | Maintainer | Review all PRs Merge Nova PRs Close Block | Adminstrator Level | Channel Access, Moderate Dev Channels, Grant Roles | | Assistant Maintainer | Review all PRs Merge Nova PRs (with approval from a Maintainer) | Assistant Maintainer Level | Channel Access, Moderate Dev Channels | | Inactive Maintainer | None | None | Channel Access | ## Dealing with Toxicity on GitHub - If you come across any toxicity in the comments, you may hide them as offtopic. * Use your best judgement here, and discuss with the other maintainers if you are unsure. If it happens repeatedly with the same person, discuss it with a headcoder and we can take appropriate action (either a warning, or Gitban if necessary). - Do NOT delete any offensive comments though, so if we have to make a case against someone it leaves a trail. - The same thing goes for passive aggressiveness from the contributor themselves. * (like say, a salt PR that has been worded harshly/passive aggressively). ## Sensitive information in comments - If you come across any sensitive information in comments, those should be deleted. - Or if not possible, edit them and put [REDACTED] which should result in the sensitive information being expunged from the edit history. ## Useful resources (nova-specific) [How to Mirror TG PRs](https://github.com/NovaSector/NovaSector/blob/master/modular_nova/mirroring_guide.md) [Guide to Modularization](https://github.com/NovaSector/NovaSector/tree/master/modular_nova) (TG guides that apply here as well) [TG Style Guide](https://github.com/tgstation/tgstation/blob/master/.github/guides/STYLE.md) [How to properly put comments in your code](https://github.com/tgstation/tgstation/blob/master/.github/guides/AUTODOC.md) [Guide to Feedback](https://hackmd.io/@tgstation/ryo7hc32C) [GAGS Walkthrough](https://hackmd.io/@tgstation/GAGS-Walkthrough) [How to Write a Good Design Doc](https://hackmd.io/@tgstation/BkzmU9EyK) ## Discord Guidelines As a maintainer, you get permissions over development channels. You are expected to act fairly and use your best judgement here. If something is getting out of hand, you may deal with it, however, it is also recommended that you request a chat moderator too, or higher. * You also may get access to normally restricted channels as a maintainer, leaking these channels is grounds for immediate termination without appeal. We expect you to provide a generally non-toxic environment in the discord channels (standard discord rules also apply), so here are a few basic rules to enforce: - No Toxic Criticism * This is a fairly subjective rule, and you should look to use your best judgement. We want to enable criticism, but only if it is done in a well-mannered and informative way. The last thing we want is for people to feel dissuaded from contributing due to communications in the discord. * Keep channel topics - Straightforward, try to enforce channel topics. No sprites in the code channel etc. * Maintain a civil environment - These channels are not for bickering. People should keep discussions to a purely technical manner, naturally, joking and such is allowed. - If someone requires removal from these channels, you must issue them a development ban by applying the Development Banned role on discord, as well as removing any contributor-related roles too. - We operate off of a three-strike rule, which means people are given three chances before they can be development banned. This is down to you, if you feel an incident warrants an immediate development ban, feel free to do so. - Bans are generally indefinite and require an appeal.