chore: Update code review guidelines (#9307)

* chore: Update code review guidelines

* Update reviewers.yml

* Renamed "One Review Required" to "Skip Maintainer Review"

* Link to the label

* Link to correct label

.github/pull_request_template.md

@@ -26,5 +26,5 @@ Fixes/Partially Fixes #[issue number]
 - [ ] I have performed a self-review of my code, the same way I'd do it for any other team member
 - [ ] I have tested all cases I listed in the testing scenarios and I haven't found any issues or regressions
 - [ ] Whenever I took a non-obvious choice I added a comment explaining why I did it this way
-- [ ] I added the label `One Review Required` if the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'
+- [ ] I added the label https://github.com/ParabolInc/parabol/labels/Skip%20Maintainer%20Review if the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'
 - [ ] PR title is human readable and could be used in changelog

.github/reviewers.yml

@@ -1,10 +1,8 @@
 reviewers:
   groups:
-    developers:
-      - jmtaber129
+    reviewers:
       - igorlesnenko
       - nickoferrall
-      - BartoszJarocki
     maintainers:
       - mattkrick
       - Dschoordsch
@@ -13,15 +11,14 @@ reviewers:
       - tghanken
     designers:
       - ackernaut
-      - enriquesanchez
     devops:
       - rafaelromcar-parabol
       - adaniels-parabol
       - dbumblis-parabol
     none:
 
   per_author:
-    developers:
+    reviewers:
       - none
     maintainers:
       - none
@@ -33,7 +30,7 @@ reviewers:
       - none
 
   defaults:
-    - developers
+    - maintainers
 
 files:
   "**/migrations/**":

docs/codeReview.md

@@ -1,9 +1,6 @@
 # Code review policy
 
 The goal is to help reviewers to make their intentions clear to the author and for the author to know exactly what is expected of them to pass the next review round.
-Review progress is tracked in the [Sprint Board](https://github.com/orgs/ParabolInc/projects/1) by the issue the PR belongs to.
-If there is no issue belonging to the PR, then the PR itself should be added to the board.
-A PR advances from self-review to reviewer-review and finally maintainer review before it can get merged.
 
 ## Submitting a PR
 
@@ -13,9 +10,10 @@ A PR advances from self-review to reviewer-review and finally maintainer review
 
 ## Requesting Code Reviews
 
-- Self-assess which domains are involved in the changes made by your PR, find a reviewer from among the folks who fill the [Code Reviewer (L2–3)](https://www.notion.so/Code-Reviewer-L2-3-a47bb0759a0b41b5b0469ff14a8cdaae) role, paying specific attention to who GitHub suggests, who has indicated expertise within a domain (e.g. backend, front-end, etc.), and who has capacity to review within 24 hours.
+- Self-assess which domains are involved in the changes made by your PR, find a reviewer from among the folks who are in the reviewers or maintainer [groups](/.github/reviewers.yml), paying specific attention to who GitHub suggests, who has indicated expertise within a domain (e.g. backend, front-end, etc.), and who has capacity to review within 24 hours.
+  - If you feel that maintainer review isn't necessary, add the label: https://github.com/ParabolInc/parabol/labels/Skip%20Maintainer%20Review. Use the label for PRs of any size and complexity if you are confident enough, and it feels safe. One exception: we should be extremely careful with DB migrations as they might break the application in the way that is hard to restore.
+  - Otherwise request a review directly from a maintainer.
 - If the PR has not been reviewed within 24 hours, ping the reviewer on Slack and ask when they'll be able to review. If they don't have capacity for review, reassign to a different reviewer. If you're still blocked at the next check-in meeting, add an agenda item.
-- If the developer creating the PR feels that maintainer review isn't necessary, they should add a label: “One Review Required”. Use the label for PRs of any size and complexity if you are confident enough, and it feels safe. One exception: we should be extremely careful with DB migrations as they might break the application in the way that is hard to restore.
 
 ## Performing Code Reviews
 
@@ -26,13 +24,6 @@ A PR advances from self-review to reviewer-review and finally maintainer review
 - When reviewing a PR and requesting changes, be mindful that the PR author won't always have the right background to understand what are you requesting. Make your comments meaningful, and record a Loom if needed.
 - [The right balance](https://docs.gitlab.com/ee/development/code_review.html#the-right-balance)
 
-## Sprint Board
-
-- Issues can go in these columns: To Prioritize, Backlog, To Do, In Progress, Stuck.
-- Pull Requests can go in these columns: Stuck, Self Review, Reviewer Review, Maintainer Review.
-- A PR may correpsond to 0 or many issues. These issues shall stay "In Progress" and when the PR gets merged, they'll be moved to "Done" automatically.
-- The motivation for this structure is to relax the constraint that 1 issue has 1 PR. If a PR resolves 3 issues, we only have to update the status of the PR, not update all 3 issues as a group. If a PR fails to resolve 1 of the 3 issues, the remaining issue stays "In Progress" instead of being moved from "Maintainer Review" back to "In Progress". If an issue has multiple PRs, the issue can stay "In Progress" while the PRs move through the process.
-
 ## Reviewer
 
 - Prefix each comment with a label. The labels are not points and will not be summed up or similar.
@@ -48,24 +39,17 @@ A PR advances from self-review to reviewer-review and finally maintainer review
     e.g. nice work here, I learnt something, good find
 - Final review is "Approve" if there are no negative comments, otherwise "Request changes"
     - Changes requested? Move the issue associated with the PR to the self-review column.
-    - Approved? Move the associated issue to maintainer-review.
-
-## Maintainer
-
-- Maintainer follows the same process of Reviewers
-- Approved by the maintainer?
-  - If there are +1 comments, the issue the PR belongs to goes back to self-review to give the author a chance to react to the comments.
-  - If there are only +2 comments, the PR can be merged directly by the maintainer.
+    - Approved? The PR can be merged by either the author or a maintainer
 
 ## Metrics Representative
 
 - Metrics Representative ensures any analytics related changes work well with downstream data services
-- This role is currently filled by @tianrunhe
+- This role is currently filled by the [data group](/.github/reviewers.yml)
 
 ## Author
 
 - Answer or resolve each comment
     - resolve if you followed the suggestion
     - reply if you didn't
 - If you need to clarify parts of the code, check if it can be done by adding comments or improve naming of variables/functions/classes
-- When you replied or resolved all comments, move the PR back to review
+- When you replied or resolved all comments, re-request review so the PR shows up in the review requested search