Recently, I met with one of my direct reports, and I gave them positive feedback about a comment they made in a Github pull request (PR). Their response was one of total surprise: None of their managers had ever looked at their PRs before! Did this mean I was checking up on their code? Because, honestly, they told me, this made them feel a bit micro-managed.
I assured them I wasn’t secretly grading their code. But the conversation did make me wonder: Why do I read my teams’ PRs? I always subscribe to updates when my teams spin up a new repository, but what am I hoping to accomplish by watching all those updates go by? Now that I’m a manager, am I hoping to stay in touch with my inner engineer who spent 15 years writing code? Am I hoping to learn more about new languages, like Elixir and React, with which I have little direct experience?
After some reflection, I realized exactly what I was doing: I was keeping my finger on the pulse and health of my team. In fact, PRs can tell a manager a lot about the health of their team. A strong and effective feedback process can have tremendous effects on a team’s morale, trust, and communication, as well as on their development velocity.
Here are seven critical insights I learned by reading my team’s PRs and what these insights reveal about how they work together:
- Our PRs are polite. When reviewers on my team leave comments about their peers’ code, the language they use demonstrates helpful respect—never critical snark. When a reviewer suggests a change, they’ll lead with something like, “Have you considered? …” and never something like, “This is totally wrong and needs to be changed—what were you thinking?!?” This approach makes it clear that my team’s engineers respect each other.
- Our PRs don’t include many back-and-forth exchanges. We have a practice that an engineer doesn’t pick up a development task until the work is well defined—typically, as a story in JIRA. When another engineer reviews the PR attached to that task, they’re free to think: “This isn’t the way I would have done it.” But if the PR contains code that fulfills the work’s acceptance criteria, the reviewer accepts the PR. When I see reviews like this, I know we’re doing a good job grooming our stories, and as a result, our team members are on the same page.
- Our PRs are readable. Team members are diligent about pointing out opportunities to improve the readability of submitted code, which I really appreciate. Even though I don’t always know exactly what the code is doing, I can follow along because the method and variable names are specific and clear; and because they call out odd behavior with helpful comments. Readable PRs allow engineers to move forward with confidence as they continue to iterate on the code.
- Our PRs are well-tested—by robots. If I see dozens or even hundreds of lines of changes in our code, but no new unit or integration tests, I take that as a warning sign that we may be accruing technical debt or heading towards “bug-town,” which could lead to slowdowns. By reviewing the PRs, I know my teams are including the necessary automated tests where appropriate.
- Our PRs are well-tested—by humans. Comments like, “This doesn’t meet acceptance criteria” or, “We didn’t call this out in grooming, but this edge case doesn’t make sense to me” let me know that my teams aren’t just turning the crank on PRs; they’re also working to ship quality software.
- Our PRs contain only the needed changes. We keep our PRs as small as possible, which makes it easier to troubleshoot our code and track down bugs when necessary. For example, I’ll often see one PR for a refactor—and then another PR hot on its tail will take advantage of the new code structure. And because our PRs contain only specific and scoped changes, it’s much easier for reviewers to identify potential bugs in that new code.
- Our PRs add instrumentation. Finally, as New Relic engineers, we drink our own champagne. By adding instrumentation in every PR, we don’t just return error conditions; we use the New Relic platform to record and alert ourselves to problems developing in our services. This makes us confident that we’ll respond to incidents quickly and effectively, no matter when they occur.
The ability to write good PRs depends on spoken and unspoken expectations. There have definitely been moments when, as a team, we’re taken a step back in order to right the ship—whether it was because we weren’t doing a good job of testing before we went to staging, or because we were sneaking extra work inside a PR. But since I’ve been with these teams, we’ve never needed to discuss how we treat each other in PRs—because every team member believes that the best path forward is one that follows the Golden Rule.
Every manager dreams of leading a happy, productive, and high-functioning team. And while I would love to take credit for my team’s success, I know their work together is actually a testament to New Relic’s engineering culture and practices. And I find it very rewarding to watch, and to participate in, the organic growth of a healthy coding culture!