Human-Centered Pull Requests: What Most PR Advice Is Missing
Engineering more than just software in Pull Requests
Relationship Engineering
From the outside, software engineering might seem like it’s all technical nerdery, but it’s ultimately a social endeavor.
And just like in any social endeavor, relationships are everything. Your team's software patterns, delivery pipelines, design docs, and pull requests all need to pass through a complex network of relationships before becoming meaningful to stakeholders. Before that, they're just bytes sitting on some disks.
Yes, yes, we’re throwing some brainy-ass instructions at chips in the cloud, but to me, that feels more like an implementation detail of the real work at play – collaborating as humans to exchange information, elevate each other, and reach our goals as a team.
The Divine Diff
As software engineers, the pull request is arguably the primary way we engage each other’s work on a daily basis, so it’s important to put thought and effort into how we engage this most sacred process.
I’ve developed some opinions over the years about what makes pull requests effective. But since technical advice on PR mechanics has been talked about to death, I figured it’d be fun to share tips on authoring and reviewing PRs with a focus on human relationships and healthy collaboration. There are some practical tips in here too though, so let’s dive in!
Fork With Feelings
Authoring pull requests is an art. Here are some tips I consider the most useful for contributing to healthy team culture and good vibes.
1. The Real Why™ should be be clear
The Real Why™ behind a pull request usually isn’t as technical as code. And there are probably humans involved. And there’s likely a story at play, even if it’s boring af 😂. It’s usually somewhere between the technical why (mrah) 🤓 and the business why (need moar revenue)💰.
Getting spun up on The Real Why™ quickly should be easy for the reviewer. A well-disciplined shop would probably have this captured in a ticket but it’s not inconceivable for a PR to be submitted without one. If that happens, it’s up to the author to cover this base. If a PR could never exist without a ticket in your shop, I commend your engineering rigor!
In either case, let’s look at an example. We’ll examine a description of why a PR is submitted that upgrades our current version of a software component called external-dns
:
Our current version of
external-dns
relies on deprecated k8s APIextensions/v1beta1
, which is removed in k8s1.22
.
Is this an adequate description of the why behind the change?
Maybe. It’s a technical description of why but in my opinion, not the Real Why™
Here’s how I’d characterize the Real Why™:
For our k8s-hosted sites to be accessible to users, DNS needs to work. DNS will stop working if its management component isn’t compatible with our version of k8s, and we’re scheduled to upgrade to an incompatible k8s version in Q3.
Notice the difference between the technical why and the Real Why™. The Real Why™ explains why people should care, not why the code is the way it is.
The technical why is still important. But between the two, I’ve seen the Real Why™ omitted more frequently than vice versa. Your reviewer will appreciate quick access to the Real Why™, especially if they can see you put effort into making it available to them.
(btw, getting good at writing Real Whys™ is a useful skill to develop as an engineer because it forces you to think about outcomes, which is ultimately why we’re gettin’ paid!)
2. The PR should be as small as reasonably possible
PRs should be as small as you can reasonably make them. This is so important.
Luckily, this is an engineering community best practice at this point. Google has a great writeup on why small PRs are best which captures most of why this is worth putting into practice. But I also felt it’s worth adding some additional thoughts as they relate to this blog post’s theme.
What’s not as explicit in Google’s guidance is that smaller PRs are more considerate to your reviewer.
Tagging your teammate in a thirty-line PR doesn’t ask them for the same time commitment or expensive headspace they’d need to be in to review a 700-line PR. It’s more respectful. You’re asking them to take a quick look at your work, not to help you move 😂. And they’ll feel that respect when you take the time to break the changes down into smaller chunks. They might not even consciously realize they’re respecting you for it, but they will. It’s a small thing but these small things add up.
The theme we’re diving deep on is that building strong relationships at work will enable you to deliver more value, faster, and strong relationships are usually forged from many small interactions over periods of time. How you choose to signal respect to your peers over time significantly influences how much harder or easier your job will be.
3. Always assume positive intent from review comments
Always assume positive intent.
This adage is so profound that it deserves an entire blog post. It’s the number one best piece of advice I’ve ever received in my entire career, and applies to pretty much every type of interaction, not just PRs.
Ultimately, you can’t know anyone’s motivations or intentions other than your own, so trying to extrapolate them by reading into comments is pointless. Don’t take anything personally. Don’t assume someone’s being lazy. Don’t assume someone doesn’t care about quality. Don’t assume someone is just trying to show off. None of that type of stuff!
If you assume any form of ill intent, you risk acting on being wrong and/or unnecessarily carrying negative feelings around with you. The only realistic outcomes are work becoming more challenging and/or feeling bad. Why bother? It’s a net negative in all but the most extreme circumstances.
My advice: assume everyone is trying their hardest to help you. It costs nothing and it only helps you. Half the time the assumption will be right, even if your intuition would have led you astray. And if you’re wrong… who cares?
What’s more important: passing rightful judgment on someone or giving yourself what you deserve?
4. Acknowledge every comment
If someone takes the time to make a suggestion or offer guidance, you should acknowledge their comment and signal your intention. For every comment. I imagine could be a controversial suggestion but I stand by it because the ROI seems clear as day to me.
These are the three main ways to signal how you intend to respond to a suggestion:
Update your code to account for the reviewer’s guidance
Reply with your intentions in a comment
Use emojis. They work great! They’re particularly useful when the suggestion is small
There should be a 1:1 review comment to some form of acknowledgement from you. The thing you want to avoid is leaving the reviewer with no way to know what you intend to do based on their suggestion – or wondering if you even read it (you also risk this when you rebase mid-review – avoid doing that!).
This is important even if the comment is subtle. Here’s an example of a comment:
Tiny nitpick, feel free to ignore: Maybe consider using 'fetch' instead of 'get' here. It might be a bit more semantically accurate, but it's definitely not a big deal.
Even though the reviewer gives you a free pass to ignore this comment, I wouldn’t. They cared enough to share – care enough to let them know how the comment will influence you:
I think you’re right @my-valued-peer but I’m gonna pass for now. I’d need to correct other unrelated codepaths to maintain consistency and I’d rather avoid regression risk on this specific task. Will bear this in mind though!
That’s just me. You could even just do:
Thanks. Think I’m gonna pass this time around
It’s a few extra seconds of work but respecting someone enough to acknowledge every comment they offer has much more of an impact than it might seem. There’s a subtle, subconscious connection, appreciation, validation, and ultimately relationship building at play. And again, relationships at work are the key channels for making things happen, delivering value, and hopefully achieving some level of job fulfillment. It’s small but these things add up over time!
Reading Between the Commits
Let’s move onto the role of reviewing code. Reviewing PRs is hard work if you’re doing it right. It’s a skill. But it’s a skill that, if cultivated, can elevate your career and help you stand out as an engineer.
1. Keep in mind your #1 goal: Helping your peer move forward!
This is numero uno, y’all!! I simply can’t emphasize this enough.
We’re all in the zone at work. We’ve got a bunch of nerdy computer shit on the brain and we all think very highly of our own technical skills. Good! We should! We’re competent, thoughtful engineers. ❤️
But let’s take a step back for a moment. When you get tagged for a PR review and you pull up the code, what’s your primary goal? Think hard about this!
If you answered something like “ensuring the code change is aligned with org engineering standards and best practices” or something to that effect, you’re not wrong but I’d encourage you to take a step even further back.
I’d argue that, when reviewing a pull request, “ensuring the code change is aligned with org engineering standards and best practices,” is how you achieve your goal, but the higher altitude goal is helping your peer reach their goal. You’re a stakeholder in their success. You’re on a team with goals and you’re pulling in the same direction to make things happen. That’s ultimately the thing that should be front and center in your brain, influencing the content and tone of your comments more than anything else.
In my opinion, it’s worth putting effort into signaling your understanding of this. If you do this (and other things like this over time), it will help your relationships, which, again, over time, will contribute to making yours and everyone’s job easier.
So my question to you is this: when the PR author reads your review feedback, would it be obvious to them that your goal is to help them reach theirs? How would you express that? This isn’t something you have to do, but why pass up such an easy opportunity to make your job incrementally easier (if not spreading positivity into the world)?
Commenting thoughtful suggestions that provide more than critique but partnership on the solution is a great start. Or, if there’s a gap between the code in the PR and where it needs to be, offer to help out-of-band from the PR.
The impact of your tone also can’t be overstated. The way you say something can say a lot about your respect for your peer. Code is incredibly dry, so comments can be inherently emotionless. Take a second to inject some positive energy into them. That’s what emojis are for! Surprise your peer with the thought “oh, this person really respects me.” Give someone a reason to want to help you with your goal later. What do you have to lose?
2. Point out good things
I can think of at least two great reasons to put effort into pointing out positive things in the pull requests you review.
For starters, as I’ve alluded to, in my opinion, pull requests are an opportunity to build relationships (shocker). I get that might seem like a stretch to some, but hear me out!
Taking time to express gratitude or appreciation for your peer’s work is a great way to connect with them as a human being. Show them you appreciate them, not just their work. Doing this not only makes the world a more positive place to live and work in, but will help you achieve your goals down the road since again, it’s hard to be an effective engineer without leveraging relationships.
Let your peer know you learned something from them! Let them know you thought their approach was clever! Let them know you were really impressed with how they handled an error you wouldn’t have thought of!
Wow, I wouldn’t have thought of this!
Comments like these mean a lot to people. You’d be surprised how impactful adding a comment that says “Neat!” could be making things easier for your team – even if subtly.
Secondly, and maybe more obviously, it’s a great way to reinforce engineering standards, good patterns, good behavior, etc.
Love to see unit tests for this use-case
Positive reinforcement works. Let’s go fam!!!
3. Contextualize your comments
While reviewing, making your expectations clear in your comments takes a lot of burden off the author, and ultimately makes for smoother communication and vibes. Things like “minor nit” or “potential blocker”, or “just a suggestion” are great. Just try to make it so that there’s little ambiguity about what you expect to be done in response to your comment.
At Thoras.ai, some of us use Netlify feedback ladders. It’s been really useful. Conventional Comments is another variation that I just learned about recently and seems great.
4. Manage review time professionally
Lastly, being responsive to review requests sends strong signals about a lot of things. It sends a message that you’re engaged with the team. It signals you’re a stakeholder in your peers’ success. It signals you take your job seriously. It signals you care! All of these things go a long way towards forging great relationships at work.
At the same time, dropping what you’re doing every time you’re tagged for review isn’t realistic – you still need focus time for deep work. It all comes down to being intentional about your time. Here are some tips:
Respond quickly to review requests when you can. If it’s a small PR and you’re not in the middle of deep work, just knock it out and unblock your peer. In between meetings? Insta-review on request. You won’t be getting much done with only 30m anyway.
Use VCS / Messaging integration to be notified about reviews you’ve been tagged on in real-time. Here’s an example of my GitHub / Slack settings for getting DM’d in Slack:
Allocate a daily time budget for reviews. Managing your own expectations about how much of your day will be dedicated to reviews should help with being interrupted. It also lets you give yourself permission to be done with reviews for the day.
Designate certain times of the day for your review backlog or larger PRs. Use a recurring calendar invite. For example, using GitHub / Slack integration settings to message you in the morning with pending reviews and using the morning for catching up.
Disable distractions. Turn off Slack and silence your phone! When you’re reviewing code, give it your full focus. It maximizes your opportunity to have an impact, not just on the codebase but on your relationship with the author
Conclusion
Relationships are everything – even at work. Viewing our day-to-day interactions opportunities to cultivate them (even in something as dry as pull requests) will pay off in dividends in your career and maybe even your life as a whole.
We’re shippin’ fam!! 🚢