Submitting Pull Requests: Who to Tag

Owned by Adam Palay (Deactivated)
Last updated: Apr 03, 2023 by Feanil Patel
2 min read

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:

  1. If it's very obvious that someone should review the code, tag that person
  2. Otherwise, run this script to figure out whom you should tag: https://gist.github.com/adampalay/7fb189284f3d215369b264f004de11c6

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-39 - Getting 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.