Managers: Look at Your Engineers’ PRs
Tags: management, codequality, testing, discuss
Published: 2020-05-24 06:20:08

While managing my last team, I focused on the basic unit of work for an engineer — the pull request. My goal was to improve the quality of my feedback by referencing the engineer’s work directly and deriving concrete action items. I mainly looked at PRs that were already merged. My goal wasn’t to be a code reviewer by checking the PRs for correctness, it was to learn more about the engineers on my team through their work. How did they approach problems? Did they care about testing? Do they respond to code review feedback well? Do they save their code reviewers time by adding clear descriptions and comments?

An engineer’s code demonstrates more than their computer science fundamentals. An empathetic engineer thoughtfully names their variables for readability, a humble engineer tests their code, and an engineer-owner reviews her teammates’ code like it’s her own.

## Identify Exceptional Team Players

PRs helped me identify exceptional engineers.

I worked with an engineer who delivered his projects on time, participated in our team meetings, and volunteered to fix abstruse bugs. He was a solid engineer. But his PRs revealed hidden talents. In code reviews, he leveled-up his teammates by nudging them to add unit tests, rethink function names, and clean up duplicate code. In some of his PRs, he cleaned up stale code and fixed flaky unit tests written by other teams. He wasn’t just a solid engineer, he was a rockstar. A humble, empathetic owner. He never mentioned his heroic code cleanup activities during standup. And peer feedback was incomplete because it came from engineers on the same team, not engineers from different teams. If I hadn’t looked at his PRs, I would’ve missed his best qualities.

Another engineer on my team was skilled at identifying non-performant queries, and another was skilled at condensing multi-line blocks of JS code into a single line with the help of nifty JS libraries.
You may also find that some engineers are consistently not testing their code or responding to valid code review comments. If so, then congratulations! You found clear, actionable feedback to give that engineer.

PRs are also a clear indication of when and how people work, and whether there are systemic problems on your team. If PRs are consistently opened after work hours, it could indicate that your team has too many meetings or a workspace that’s not conducive to productivity. If there’s a sharp decrease in the number of PRs delivered by a given engineer, it may be a sign that he or she is burned out. If you suspect that an engineer on your team isn’t pulling their weight, start by looking at their PRs. Their PR output may be low, but their contribution as a code reviewer may be high.

While scanning PRs, find opportunities to recognize engineers for contributing to another engineer’s growth. This is an effective way of keeping your engineer’s goals aligned around the team’s success instead of solely on their own success. Thank engineers for their thoughtful code review comments and detailed PR descriptions. Show them that you notice and value the time they spend reviewing code for their teammates. Show them that helping a teammate’s project advance bug-free is worthy of gratitude and recognition.


## Tell Your Team

This part is optional. You don’t have to tell your team you’re reviewing their PRs, but I did and found it beneficial. I told the engineers in my team that I was checking PRs to derive feedback and to learn more about them.

Some engineers on your team may be uncomfortable with you checking their work. I would argue that this is okay. In fact, I’d argue that it is a part of your role as a manager to learn about how they work, whether or not they are comfortable with it. And how they work is largely demonstrated by their code. If you feel wrong about doing this, remember that there might be unsung heroes on your team who do the right thing when no one’s looking. You need to look. If an engineer knows you’re checking their PRs, and it inspires better code and documentation from them, that’s a positive outcome for your whole team.

## Performance Management

By performance review time, I had gathered so much data through PRs that it became straightforward to justify the performance scores I’d given my team. Code is concrete and PRs are concrete — they’re impossible to argue against. And by regularly reviewing an engineer’s PRs, you’ll form a complete picture of who they are as an engineer. You’ll have everything you need to create a performance narrative, including the data to back it up.

Note that as engineers become more senior, they tend to code less. They participate in post-mortem reviews, architecture reviews, and product discussions that are higher leverage than shipping code. But the PR is still relevant. Are senior engineers reviewing the code of their more junior teammates? Are they demonstrating their expertise by catching edge cases and encouraging best practices? When evaluating senior engineers, it’s important to evaluate their code review quality.

## Keep in Mind

Zooming in has its downsides.

Looking at all the PRs of all your engineers will take a lot of time. To make the review process more efficient, I focused on PRs for major features and PRs with comment activity. I didn’t look through every line of code, I skimmed the description, comments, and generally scanned the PR for signs of empathy-driven development. I spent around 2 hours in total per engineer per month reviewing PRs.

You may be tempted to form an opinion about an engineer from looking at a single PR. Beware of quick judgments! Every human being has their good days and bad days, their exhausting days and energetic days. Don’t let one substandard PR morph your opinion of the engineer. It is only one data point of the many data points you should be collecting.
Thank you for reading! I would love to hear your thoughts and ideas about other ways to deliver concrete feedback and identify unsung heroes on your teams. Please comment below.