Shipping faster with a new PR processing system

Nick Zheng | Pinterest engineering manager, Infrastructure

As our engineering organization quickly grows, we developed a formal process to ensure every piece of code has clear owners and all code changes made on critical paths are approved by corresponding owners. This improves long-term code quality, health and development velocity and enables us to be a more effective and collaborative team.

With this process developed, we configured our code review system (Phabricator) to assign requests to project owners. However, there are some code paths many engineers contribute to that only have a small number of owners. This is a problem, because everyone receives all of the related review requests, resulting in unnecessary notifications for each review update. Some owners of a busy code repository may receive 40–50 review requests, as well as all of their updates, everyday.

The above diagram shows how frustrated each project owner could be due to the massive code review requests/notifications.

As we continue to scale the engineering organization, this situation becomes worse and prevents us from moving fast. We knew there had to be a better way, so we implemented a new system that round-robins PRs (code review requests) among team members, so each PR is automatically assigned to one owner, and each project owner only receives 1/n of all code change PR notifications for a project (n is the number of the total project owners).

The above diagram shows how the new process dramatically improves the situation for each project owner.

Building a round-robin code review system

Before, we used Phabricator’s Herald rule to send notifications of related changes to project owners, but it didn’t have the functionality to round-robin PRs among project owners. We had to extend the Herald rule to do that.

We save the list of project owners in a particular order to the database when such a Herald rule is triggered at the first time by a PR. The first owner from the list is selected, assigned the PR and put to the last position of the list. Now the original second owner moves to the first position to become the first owner waiting for the next PR. When the next PR is coming, it repeats the same process, as show in the below diagram.

If we update the project owners, the list will also be updated when a new PR triggers the Herald rule. A new member will be inserted in the list, and a removed member will be taken off.

The following diagram is the newly extended Herald rule.

As shown in the above diagram, both “Add a blocking reviewer in the Round-Robin way” and “Add a non-blocking reviewer in the Round-Robin way” are new actions that the extended Herald rule provides.

This approach provides a new way to programmatically assign code review requests to the project owners, and ensures the project owners are more productive and efficient. It enables our code ownership to be more effective and scalable, and ultimately helps us move fast.

Acknowledgements: The extended Herald rule was implemented by Nick Zheng, and the idea of a round-robin system for PRs amongst project members originally came from Steve Cohen on the Infrastructure team.