Skip to content

Commit

Permalink
[IOSP-607] Update PullAssigners document to remove reference to Pull …
Browse files Browse the repository at this point in the history
…Panda and explain GitHub's code review assignment (#359)

* Updated intro.

* Updated Teams config.

* Removed section about proxy team.

* Removed section about configuring Pull Panda.

* Remove setup section.

* Updated CODEOWNERS section.

* Clean up.

* Added section about enabling GitHubs code review assignment.

* Clean up.

* Removed reference to Pull Assigner being flaky. GitHub is never flaky.

* Clean up.

* Clean up.

* Update Cookbook/Technical-Documents/PullAssigners.md

Co-Authored-By: Konrad Muchowicz <[email protected]>

* Update Cookbook/Technical-Documents/PullAssigners.md

Co-Authored-By: Konrad Muchowicz <[email protected]>

* Updated Out of Office docs for GH.

* Tweaks.

* Update Cookbook/Technical-Documents/PullAssigners.md

Co-Authored-By: Olivier Halligon <[email protected]>

* Update Cookbook/Technical-Documents/PullAssigners.md

Co-Authored-By: Olivier Halligon <[email protected]>

* Update Cookbook/Technical-Documents/PullAssigners.md

Co-Authored-By: Olivier Halligon <[email protected]>

* Mentioned routing algo in CODE_REVIEW.

* Mentioned teams in NewHiresCheckList.

* Update Cookbook/Technical-Documents/PullAssigners.md

Co-Authored-By: Olivier Halligon <[email protected]>

* Included teams in ToolsAndServices with links and added links to NewHiresCheckList.

* Grammar

Co-authored-by: Konrad Muchowicz <[email protected]>
Co-authored-by: Olivier Halligon <[email protected]>
  • Loading branch information
3 people authored Mar 12, 2020
1 parent e0d15dc commit 67b5b03
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 61 deletions.
2 changes: 0 additions & 2 deletions Cookbook/Technical-Documents/Development-Process.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ There are 3 services we use daily:
- If you'd like feedback before the work on the ticket is done, open a draft PR so your peers can leave comments
- To run checks before merging, use the comment `@ios-bot-babylon test_pr` ([more info](https://github.com/babylonhealth/ios-playbook/blob/master/Cookbook/Technical-Documents/SlackCIIntegration.md))

_Note: Sometimes Pull Assigner is not triggered when a new PR is opened. In that case, re-request `ios-pullassigner` to add reviewers from the team._

## 5. Merging PR 🚦

- Once a PR has 2 approvals and no outstanding comments/changes requested, the `Merge` label can be added to begin the merging process
Expand Down
4 changes: 3 additions & 1 deletion Cookbook/Technical-Documents/NewHiresCheckList.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ You can find the list of services you'll need and how to get access to them in [

## GitHub Access

Prior to starting, make sure you have a GitHub account (either your personal or new for Babylon) and that you have access to the following repositories:
Prior to starting, make sure you have a GitHub account (either your personal or a new one for Babylon) and that you are added to the [iOS](https://github.com/orgs/babylonhealth/teams/ios) and [iOS-Devs](https://github.com/orgs/babylonhealth/teams/ios-devs) teams. This will give you access to all the appropriate repositories and will include you in GitHub's automatic code reviewer assignments.

Once you've done that, double check that you have access to the following repositories:

- [babylon-ios](https://github.com/babylonhealth/babylon-ios)
- [ios-charts](https://github.com/babylonhealth/ios-charts)
Expand Down
17 changes: 8 additions & 9 deletions Cookbook/Technical-Documents/OutOffOfficeRequest.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,14 @@ When we leave on holidays we should make sure we leave our work either finished
1. Have a description of what is left to be done.
1. Who to contact if necessary.
1. If it's something urgent should be assigned to another team member and a proper handover should be done with that person.
1. [Add yourself to the PullReminder exclusion list](#steps-to-be-added-to-the-pullreminder-exclusion-list) to prevent you to be added as a PR reviewer while you are away.
1. Once you get back, you have to remove yourself from the PullReminder exclusion list. Adding a Slack reminder (`/remind me to reactivate PullReminder on <date>`) before you leave might help you not to forget.

#### Steps to be added to the PullReminder exclusion list
1. Go to [pullreminders.com](https://pullreminders.com)
1. Sign in
1. Select Babylonpartners organization
1. Select iOS-PullAssigners team
1. Add yourself to the Excluded team members
1. [Set your status to "Busy" on GitHub](#steps-to-be-added-to-the-pullreminder-exclusion-list) to prevent yourself being added as a PR reviewer while you are away.
1. Once you get back, you have to clear your "Busy" status. Adding a Slack reminder (`/remind me to clear my GitHub "Busy" status on <date>`) before you leave might help you not to forget.

#### Steps to set your status to "Busy" on GitHub
1. Go to [GitHub.com](https://github.com) and sign in.
1. Click your avatar in the top right.
1. Click "Set status".
1. Tick "Busy" and confirm by clicking "Set status".

## Working from Home 🏡
This section is only valid for non-remote employees.
Expand Down
82 changes: 37 additions & 45 deletions Cookbook/Technical-Documents/PullAssigners.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,72 +2,64 @@

## Introduction

[Pull Panda](https://pullpanda.com/) is a set of tools for automating actions on GitHub Pull Requests and provide some integration with Slack. Amongst those tools, Pull Assigner will allow you to auto-assign people from your team as reviewers of Pull Requests, typically trying to distribute the PRs amongst the team evenly
The aim of this document is to explain how to configure GitHub's automatic code review assignment in our Babylon repos in order for it to assign people to PRs automatically.

The aim of this document is to explain how to configure `Pull Assigner` on our Babylon repos in order for it to affect people to PRs automatically
- We will use the `CODEOWNERS` file to make GitHub always assign a GitHub team to all our Pull Requests.
- GitHub will then pick N reviewers from that team ([see below how to configure that number N](#enabling-githubs-code-review-assignment)) and unassign the team itself.
- Optionally, we could also require each PR of a repository to always have an approval from a subset of people (the ones owning the responsibility of the code) – like we did for our bots repository owned by the Developer Experience squad.

- We will use the `CODEOWNERS` file to make GitHub always assign a special GitHub team (called the proxy team in Pull Assigner's parlance) to all our Pull Requests
- That special GitHub team will in fact be managed by Pull Assigners, which will detect when that team is added as a reviewer
- Pull Assigner will then pick N reviewers from a GitHub team listing all possible reviewers, then unassign the "proxy team"
- Optionally, we could also require each PR of a repository to always have an approval from a subset of people (the ones owning the responsibility of the code) – like we did for our bots repository owned by the Developer Experience squad
## Configure a GitHub Team containing the reviewers to pick from

## Configure GitHub Teams
1. First, you need a GitHub team containing the list of all the people you want GitHub to pick reviewers from.

### A team to list the reviewers to pick from
* For the iOS repos, we use the team `@babylonhealth/iOS-Devs`, which should contain every iOS developer at Babylon. So if you need the same list of reviewers, you can just use this one instead of creating one.
* If you need a different list to pick your reviewers from, you'd need to create a team via [this page on GitHub](https://github.com/orgs/babylonhealth/new-team), and once it's created, go to the team's page and add Members to that new team ([e.g. here for `iOS-Devs`](https://github.com/orgs/babylonhealth/teams/iOS-Devs/members))

1. First you need a GitHub team containing the list of all the people you want Pull Assigner to pick reviewers from.
2. Next, **make sure that this GitHub team containing the people you want as reviewers– has access to your repository**, via the "Repositories" tab in the team's page ([e.g. here for `iOS-Devs`](https://github.com/orgs/babylonhealth/teams/iOS-Devs/repositories))

* For the iOS repos, we use the team `@babylonhealth/iOS-Admin` for this, which should contain every iOS developer that are on our team. So if you need the same list of reviewers, you can just use this one instead of creating one.
* If you need a different list to pick your reviewers from, you'd need to create a team via [this page on GitHub](https://github.com/orgs/babylonhealth/new-team), and once it's created, go to the team's page and add Members to that new team ([e.g. here for `iOS-Admin`](https://github.com/orgs/babylonhealth/teams/iOS-Admin/members))

2. Next, **make sure that this GitHub team –containing the people you want as reviewers– has access to your repository**, via the "Repositories" tab in the team's page ([e.g. here for `iOS-Admin`](https://github.com/orgs/babylonhealth/teams/iOS-Admin/repositories))

### The proxy team to trigger Pull Assigners

1. Now, we will need another, separate, GitHub team –called the "Proxy team" in Pull Assigner's vocabulary– which will be the team that you'll assign to your PRs via `CODEOWNERS` to trigger Pull Assigner on those PRs (†)

* For the iOS repos, the proxy team we use –to assign people from the `@babylonhealth/iOS-Admin` to our PRs– is called `@babylonhealth/iOS-PullAssigner`, so if you are using the same list of people from `iOS-Admin` you can also use the `iOS-PullAssigner` for your proxy team directly without creating a new one
* If instead you created a separate team because for your case the list of people in `iOS-Admin` wasn't matching the people you wanted to assign PRs for your repo, you'll need to [create a new GitHub team again](https://github.com/orgs/babylonhealth/new-team), but this time leave it with no member in it (since it will only act as proxy)

2. Next, **make sure that the proxy team has access to your repository**, via the "Repositories" tab in the team's page (e.g. https://github.com/orgs/babylonhealth/teams/ios-pullassigner/repositories)

> _(†) Side note: This proxy team is not strictly necessary, but if you don't use a proxy team and instead use the team you just declared above directly, that means that everybody will be notified and requested review every time a new PR is created. With proxy team Pull Assigners will automatically replace it with only N people from that team requesting review only from them. This is why using a proxy team without any member in it to trigger Pull Assigner is usually preferrable_
## The `CODEOWNERS` file

### Configure Pull Assigners with those GitHub teams
Now we need to ensure that the right team (e.g. `iOS-Devs`) is automatically assigned as reviewer on all new PRs.

If you are reusing the `iOS-Admin` and `iOS-PullAssigner` teams for your repo setup, you have nothing special to configure on https://pullreminders.com as those GitHub teams are already configured in Pull Assigner; so you can skip to the next section.
For this, you just need to create a `.github/CODEOWNERS` file in your repo with the following content (if you have a `CODEOWNERS` file already at the root of your repository, we advise to move it inside a `.github/` directory to clean up your repo root).

But if you have created new teams (team listing reviewers + proxy team) you'll need to log in to https://pullreminders.com and add the GitHub teams to Pull Assigner's configuration via [this screen](https://pullreminders.com/installs/6124714/assigner)
```
# Global rule for the whole codebase.
# GitHub's Code Reviewer assignment feature will only pick N people from that list
# to assign them as reviewers. To configure this, go to your Team's page
# (https://github.com/orgs/babylonhealth/teams/<yourteam>) then to
# Settings > Code review assignment
- Click "Add Team"
- Select the real (not proxy) team you've created before in the dropdown menu
- Configure the team in the next screen, especially the number of reviewers to assign, the algorithm, but also the proxy team you previously created, and select to delete that (proxy) team review request after assigning reviewers (†).
* @babylonhealth/iOS-Devs
```

(†) Note that if your repository's protected branch is configured with "Require review from Code Owners" checked, then any member or team mentioned in your `CODEOWNERS` file will stay assigned as reviewer. Which means that in that case, your proxy team, being typically mentioned in your `CODEOWNERS` to always be auto-assigned, won't be removed from your PR even if you chose "Delete after assigning reviewer(s)" for the "Team review request" setting in your https://pullreminders.com setup. It will only be able to be deleted if the "Require review from Code Owners" setting is unchecked in your repo.
## Enabling GitHub's code review assignment

## Update GitHub's PullAssigner app settings
Once the team and `CODEOWNERS` file has been created, we can go ahead and enable GitHub's reviewer assignment feature.

⚠️ This step will require the assistance of someone from `#devops` who has access to the GitHub's *organization* settings.
1. Head to the organisation's [home page](https://github.com/babylonhealth).
2. Go to the "Teams" tab and search for your team ([e.g. here for `iOS-Devs`](https://github.com/orgs/babylonhealth/teams/ios-devs)).
3. Go to the "Settings" page (to access / edit your team's settings you'll need to have the `Maintainer` team role).
4. In "Settings" go to "Code review assignment".
5. Tick "Enable auto assignment" and configure the settings as desired.

Ask `#devops` to go to the GitHub organization's settings and under the "Installed GitHub Pages" [here](https://github.com/organizations/babylonhealth/settings/installations).
**Note:** You'll probably want to tick "If assigning team members, don't notify the entire team" to prevent the entire team from getting notifications for every PR, even if they haven't been individually assigned to it.

From there they should be able to edit the "PullAssigner" app's settings and add your new repo to the list of repositories the PullAssigner GitHub app has access to
Please see [GitHub's documentation](https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/managing-code-review-assignment-for-your-team) for more in-depth details around configuring these settings.

## The `CODEOWNERS` file
#### In summary

Now we need to ensure that the proxy team (e.g. `iOS-PullAssigners`) is automatically affected as reviewer on all new PRs.
Now that we have a team of reviewers, your project has a `CODEOWNERS` file and GitHub code review assignment is enabled, the following will happen when you create a PR:

For this, you just need to create a `.github/CODEOWNERS` file in your repo with the following content (if you have a `CODEOWNERS` file already at the root of your repository, we advise to move it inside a `.github/` directory to clean up your repo root)
1. The team defined in `CODEOWNERS` will be automatically assigned as a reviewer.
2. GitHub will instantly pick N reviewers and assign them to the pull request (number of reviewers is configured in the "Code review assignment" settings).
3. The team will be unassigned.

```
# Global rule for the whole codebase.
# - The empty `iOS-PullAssigner` team will act as a proxy for the PullAssigner bot.
# When a new PR arrives, PullAssigner will be triggered, which will then pick members from the iOS-Admin team
Congratulations, you now have reviewers for your PR and you didn't need to lift a finger.

* @babylonhealth/iOS-PullAssigner
```
#### Excluding yourself from auto assignment

Of course, if you created a separate proxy team instead of using our common `iOS-PullAssigner` GitHub proxy team, adapt the content accordingly
There are some scenarios where you'd like to prevent yourself from being automatically assigned as a reviewer, e.g. holidays. To do this, simply click your avatar in the top right of GitHub, click "Set status", tick "Busy" then confirm by clicking "Set status". Busy team members will not be automatically assigned as reviewers.

## GitHub PRs configuration (protected branches)

Expand Down
2 changes: 1 addition & 1 deletion Cookbook/Technical-Documents/ToolsAndServices.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
| JIRA | Tickets of tasks and issues on your sprint | You should get an invitation email to join the BabylonPartners team after the [IT Support ticket](https://supportbybabylon.atlassian.net/servicedesk/customer/portal/5/group/12/create/43) has been resolved |
| Zeplin | Screen Designs | Ask the support engineer in the iOS team channel on Slack for access to the project(s). Depending on your tribe you might need to use Figma |
| [Figma](https://www.figma.com/) | Screen Designs | Log in with Google as this should redirect to the Babylon Okta sign in. Depending on your tribe you might need to use Zeplin |
| GitHub | Repos for the app, the bots, the playbook... | [IT Support ticket](https://supportbybabylon.atlassian.net/servicedesk/customer/portal/5/group/12/create/248) to get added to the org. Make sure you include your GitHub username in the support ticket, and that your Babylon email is added to your account |
| GitHub | Repos for the app, the bots, the playbook... | [IT Support ticket](https://supportbybabylon.atlassian.net/servicedesk/customer/portal/5/group/12/create/248) to get added to the org. Make sure you include your GitHub username in the support ticket, and that your Babylon email is added to your account. Find team members with the _Maintainer_ role to add you to the [iOS](https://github.com/orgs/babylonhealth/teams/ios) and [iOS-Devs](https://github.com/orgs/babylonhealth/teams/ios-devs) teams |
| [Lokalise](https://lokalise.co/projects) | Manage translations used by the app | Use iOS team vault on 1Password to gain access |
| Confluence Spaces (iOS, your Tribe, ...) | Documentation for various processes | [IT Support ticket](https://supportbybabylon.atlassian.net/servicedesk/customer/portal/5/group/12/create/43) – You can also ask your PM or Tribe Lead which Spaces you need to be given access to. (Since we are moving away from Confluence this might not be necessary) |
| Google Team Drives | Access shared documents for which you need multiple people to contribute | Login using your Microsoft account which is used for email – You can also ask your PM / Tribe Lead which Drives you need to be given access to. You should be given access to at least `iOS` and `Consumer Apps Tribe`|
Expand Down
6 changes: 3 additions & 3 deletions Etiquette/CODE_REVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Creating a Pull Request
* explain the context and motivation, don’t assume familiarity with the history. Highlight how the goal was achieved without going too much into details - people tend not to read long descriptions and if a long explanation is needed, then a single pull request is probably not going to be enough (i.e. the change may need to go through the proposal process or design review).
* don't strip out parts of the template unless it is absolutely irrelevant to the changes (i.e. change in the build script does not need any screenshots as it does not touch UI)
* don't forget labels (note: it's better to set them after the PR is created, as GitHub sometimes fails with 405 status code and we don't know why)
* Explicitly request review from or mention team mates you specifically want to involve in the discussion (in addition to the ones that will be assigned to it automatically by PullAssigners)
* Explicitly request review from or mention team mates you specifically want to involve in the discussion (in addition to the ones that will be assigned to it automatically by GitHub)
* Self-review your changes, adding comments where you think you can get questions (it may worth adding these as code or documentation comments). Tip: if you spot small issues you can use GitHub suggestions and apply them right away
* Keep your pull requests small, ideally they shouldn't be more than **800 additions** (Danger will make a warning if this exceeds 850 additions)
* Create a draft PR for work that is in progress, but only do that if you seek for opinions on your work
Expand Down Expand Up @@ -103,8 +103,8 @@ To improve collaboration, we take time to review pull request together in small
Pull Assigners
--------

To balance the workload and avoid knowledge silos we use [Pull Assigners](https://pullpanda.com/assigner) to assign reviewers.
To balance the workload and avoid knowledge silos we use GitHub's code reviewer assignment feature to automatically assign reviewers.

- Each pull request will be assigned to three random reviewers from the team (PS: if the PullAssigners bot seems stuck, try to de-assign it and re-assign it.)
- Each pull request will be assigned to three reviewers from the team. At the time of writing, reviewers are chosen on a "round robin" basis where assignments are given to those with the least recent review requests.
- Two approvals are enough to merge a pull request (there shouldn't be pending requests for changes though and you should address any other pending questions or comments)
- Anyone else is still encouraged to review any pull request and author can request reviews from specific teammates

0 comments on commit 67b5b03

Please sign in to comment.