Motivation
In a recent retrospective (/wiki/spaces/SUST/pages/65667249), we discussed how it's not trivial to figure out who to tag in a new PR. We therefore came up with the following proposal. Since each pull request requires two "thumbs up" to merge, we like to have one thumbs up from a member of the Escalation team and one thumbs up from a member of the team who wrote the feature we're patching.
Internal Reviewers
Before we ask dev teams to review our PRs, it's advantageous to review tickets internally first. In general, it's nice to tag someone who is familiar with the area of code that your PR touches. It's not necessary though. So, when you open a new PR, do the following:
- If you have a good idea of who to tag through git blame or previous knowledge, tag that person
- Otherwise, ask either in standup or in the escalation room of who would be good to review the PR
I'd expect folks to tag 1 (max 2) internal reviewers before asking for a pull request from a team.
External Reviewers
It's less clear who to tag on development teams. We'll have more information about this soon, once this is discussed in a cross-team setting. In the meantime, try your best with git blame, otherwise:
- For ECOM, ask the bug czar (listed in the top of the ECOM room in hipchat), or AwaisQ (Deactivated) or someone else in Lahore
- Note: for small, clear issues, there's no need to tag someone on ECOM — two Escalation thumbs up are ok
- For TNL/Platform, either ask someone in Lahore first (like Usman may know who to tag for TNL), or ask in their HipChat channels.
Things to keep an eye on
We want to make sure we're not tagging the same people over and over again. Part of the point of tickets like - SUST-39Getting issue details... STATUS is to help us measure when we're burdening certain people too much and should spread reviewing responsibility a little more widely.