Menu

Reviewing Pull Requests

2022-12-19

Context:

This is excerpted from a Datadog-internal post I wrote on code review best practices.
Profiling teams are splitting up OKRs into 2-3 person squads. This experiment is going quite well modulo some pain points around code reviews. As few squads have the luxury of more than one backend or frontend engineer, we need to get better at inter-squad reviews.

Also, we’re hiring.

Reviewing Pull Requests

Reviewers and authors experience an awkward power imbalance with the scales tipped to the reviewer. Even though the author has more time invested, they are dependent on the code reviewer’s oversight.

Here are some tips to make reviews more collaborative and higher quality.

Synchronous Reviews

• Pairing is the highest bandwidth possible for code reviews. Both author and reviewer are on equal footing and are putting an equivalent effort into the change set. This is often not possible with small squads and a key downside is the high-bandwidth knowledge transfer is ephemeral.
• Review 1:1 - scheduling 30 minutes to go over a review is similarly high-bandwidth and a great tool for complex changesets; similar downsides as pairing

Asynchronous Reviews

If the review can not be done synchronously, there are still things the reviewer can do to level up collaboration and reduce the power imbalance. All of these tips move the scale on time invested and power balance between author and reviewer.

• Pull down the feature branch locally rather than read it on Github.
• Run the tests.

Create a pull request against the pull

Rather than dictate suggestions as comments, try out the changes you want in another branch.

This demonstrates respect for the author’s time. If the review feedback is a good idea, it’s easy for the author to merge or cherry-pick the suggested changes. A low-effort review comment that forces the author to spend even more time can be demoralizing and should be avoided if possible.

Demonstrating the change rather than dictating is often net-time efficient and collaborative.

This technique is an effective shallow-feedback filter. Often in trying out a suggestion, the reviewer realizes a 5-second burst of insight during the review lacks context and is sub-optimal. Not adding that bit to the review is also a net-time improvement.

The author is (hopefully) receiving constructive feedback, but a reviewer can struggle to frame feedback as either a “nit” or go nuclear and block the review with “Request changes.” The above tips can help with the framing and make the experience better for both parties.

Review styles to avoid

• the slow drip - Author incorporates feedback as the reviewer slow-drips multiple rounds of “Request changes”
• the low effort opinion - Software engineering can be an artistic act and everyone has style preferences. It’s a better experience for all parties to have a nits discussion 1:1 than in the durable record of code review comments.
• the rubber stamp - arguably worse than no review

Tooling tips

Github’s pull request UI is still :vomitemoji:.

IntelliJ IDEA has tooling for Github Pull Requests and reviews. Advantages include

• code navigation
• shortcuts for navigating the file diffs
• IDEA’s diff tooling

More resources

Chelsea Troy wrote a series on pull requests.

• how to write a good pull request
• how to create a good pull request review
• how to write commit messages

Related tags:

• resilient software engineering

site menu

• home
• posts
• notes
• tags
• rss

Back to top