Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the PR process doc to come back to one reviewer #6396

Merged
merged 1 commit into from
Jun 28, 2022
Merged

Update the PR process doc to come back to one reviewer #6396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/6396.doc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update the PR process doc to come back to one reviewer with optional additional reviewers.
12 changes: 7 additions & 5 deletions docs/pull_request.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,16 @@ Exceptions can occur:

##### PR Review Assignment

We use automatic assignment for PR reviews. A PR is automatically routed by GitHub to 2 team members using the round robin algorithm. The process is the following:
We use automatic assignment for PR reviews. **A PR is automatically routed by GitHub to one team member** using the round robin algorithm. Additional reviewers can be used for complex changes or when the first reviewer is not confident enough on the changes.
The process is the following:

- The PR creator can assign specific people if they have another Android developer in their team or they think a specific reviewer should take a look at the PR.
- If there are missing reviewers, the PR creator assigns the [element-android-reviewers](https://github.com/orgs/vector-im/teams/element-android-reviewers) team as a reviewer.
- GitHub automatically assigns other reviewers. If one of the chosen reviewers is not available (holiday, etc.), remove them and set again the team, GitHub will select another reviewer.
- The PR creator selects the [element-android-reviewers](https://github.com/orgs/vector-im/teams/element-android-reviewers) team as a reviewer.
- GitHub automatically assign the reviewer. If the reviewer is not available (holiday, etc.), remove them and set again the team, GitHub will select another reviewer.
- Alternatively, the PR creator can directly assign specific people if they have another Android developer in their team or they think a specific reviewer should take a look at their PR.
- Reviewers get a notification to make the review: they review the code following the good practice (see the rest of this document).
- After making their own review, if they feel not confident enough, they can ask another person for a full review, or they can tag someone within a PR comment to check specific lines.

For PRs coming from the community, the issue wrangler can assign either the team [element-android-reviewers](https://github.com/orgs/vector-im/teams/element-android-reviewers) or any members directly.
For PRs coming from the community, the issue wrangler can assign either the team [element-android-reviewers](https://github.com/orgs/vector-im/teams/element-android-reviewers) or any member directly.

##### PR review time

Expand All @@ -102,6 +103,7 @@ Some tips to achieve it:
- Set up your GH notifications correctly
- Check your pulls page: [https://github.com/pulls](https://github.com/pulls)
- Check your pending assigned PRs before starting or resuming your day to day tasks
- If you are busy with high priority tasks, inform the author. They will find another developer

It is hard to define a deadline for a review. It depends on the PR size and the complexity. Let's start with a goal of 24h (working day!) for a PR smaller than 500 lines. If bigger, the submitter and the reviewer should discuss.

Expand Down