PR Traffic ahead!
For a few weeks now, we’ve had a Pull Request backlog that was waaaay too long. Why? The team is acutely aware of the impact it has. Surely it’s in everybody’s interest to keep the list short? I think a healthy mix of the bystander effect and optimism bias are to blame; the pressure to get things done was slowing us down. PRs were being created atop of other PRs! PRs weren’t getting shipped because we were too busy creating PRs!! As the checklist manifesto shows — even the smartest people need process…
The problem
Our PR queue was creating inefficiencies. Over the last couple of months we’ve seen the following issues:
- Stale PRs: PRs that had gone a number of days without a review were increasing the load on context switching.
- Rebases: PRs that needed rebasing due to changes on master were taking more and more time.
- Failed tests: Test failures caused by merging dependent PRs were taking time.
- Chained PRs: PRs that can only be reviewed after dependent PRs were merged were creating noise.
So, what to do? Would process help, or some automation? At Trail we opted for a healthy mix of both to get us over the hump — and so far it’s working pretty well!
The process
A key factor in achieving success was reviewing the PR queue during our daily standups. We introduce the following guide:
- Ensure each PR has somebody assigned to review it. If not, one is assigned during standup.
- Stop people from opening PRs branched off existing PRs. If you get into this situation you should help push through the open PR.
- Review large PRs to see if they can be decomposed.
The manual process alone has made a massive difference. It helped take a queue that was often fifteen items long down to something that’s, as I type, three PRs long!
…and automation
Even with a successful manual process it’s hard to tell who a good reviewer might be, or if a PR has been branched from another. Enter Facebook’s mention-bot stage right.
In its current guise mention-bot checks to see who the most appropriate reviewers are, and namechecks them in a comment. We’ve made a minor extension to parse config from the environment. We’ve also made some changes on our fork, including a check on the PR size, and a check for branched PRs. You can fork our repo if you’d like to use these features yourself.
The solution
The daily review has removed the stale PRs almost entirely. Large PRs are being flagged and rebasing and test failures are being reduced. It’s a bit early to tell if the branched PR restriction is going to help but so far the signs are positive, woop 🎉
