6 Best Practices to Manage Pull Request Creation and Feedback

As software engineers, we sometimes get too focused on just writing code, and don’t explore the processes that help improve individual and team efficiency. Some process improvements take a long time, but there is a simple change that can make a big difference: Creating pull requests that are easy to review. 

In this article we’ll provide some best practices on how. We’ll first go into detail about how improving PRs can improve a team’s efficiency and outputs and then we’ll take a look at the best practices that have helped DoorDash improve our software development life cycle.  

Why bad PRs slow down the development cycle 

Bad PRs make it harder to collaborate, slowing down the development process. When a PR is hard to review, it gets fewer comments and disagreements, which leads to bugs and degraded code quality. Put together, these problems are ultimately much more costly than getting the code right the first time. Bugs create negative user experiences or slow time to market. Degraded code quality makes the codebase harder to build upon in the long term.

At the extreme, bad PRs can lead to “tick the box” style reviewing in which engineers approve without bothering to read the code. Negligent review defeats the purpose of creating a PR in the first place. Code developed without proper collaboration also inhibits the growth of junior developers, who need thoughtful feedback to learn. Having a robust review culture can help prevent skill stagnation. Here we outline several best practices to ensure that PRs are easy to review and serve their purpose to improve the development process. 

PR basics 

Make it easy for your reviewers by writing code that is:

1. Easy to understand
2. Well-structured and consistent with style guidelines
3. Doing what it’s supposed to
   1. Verifying correct behavior requires both checking test coverage and making sure that tests are passing for the right reasons

To help teams improve their reviewing culture, here are some guidelines to manage PR creation and feedback. We’ll dive into each of these best practices: .

1. Write descriptive and consistent names
2. Create a clear PR title and description 
3. Keep PRs short (same applies to files and functions)
4. Manage PR disagreements through direct communication
5. Avoid rewrites by getting feedback early
6. Request additional reviewers to create dialogue

Write descriptive and consistent names 

Variable and function naming is one of the biggest challenges in software engineering. 

Poor naming inevitably leads to bugs and increases the time an initial reviewer must devote to understanding the code. If poor naming passes code review, the problems with missed bugs and increased time requirements will multiply for every engineer who interacts with the code down the line. This is why it’s critical to choose good variable names at the start, saving time and preventing long-term headaches. 

We all have a sense of what good names and bad names are. It’s usually just a matter of investing the time to discuss and choose good names from the outset. 

Here are some examples: 

// Bad
val cid;
// Good
val consumerId;

// Bad
val distance;
// Good
val distance_meters;

Pay attention, too, to the larger context of how other things are named. This consideration too often is overlooked. Taken out of this larger context, names can become inconsistent or their meaning can become overloaded.

Here are some examples:

Inconsistent naming

// Bad
Service A {
    val doordash_id;
}
Service B {
    val a_id; // doordash_id from Service A
    val doordash_id; // doordash_id from Service B
}

// Good
Service A {
    val doordash_id;
}
Service B {
    val doordash_id; // doordash_id from Service A
}

Same name, different meaning

// Bad
fun function1() {
    val color_code; // means hex code
}
fun function2() {
    val color_code; // means the code column in the color table
}

// Better
fun function1() {
    val color_hex_code; // means hex code
}
fun function2() {
    val color_code_id; // means the code column in the color table
}

To avoid confusion, a variable’s name should be self-explanatory.

It’s essential to use descriptive and consistent naming for everything to avoid confusing the reviewer. This is one area where perfectionism pays off; even one instance of being lazy or tired can create confusion that leads to a bug or a frustrated reviewer.

Clear naming is crucial for the lifetime of the code. Once the code is merged, the variable/function names likely won’t change for a while. One poor naming choice can easily confuse more than 100 engineers over the course of a couple years, but once code is replicated and accepted, it can be difficult to change any names. 

Create a clear PR title and description

While many people think only about the code when they create a PR, context is essential for understanding and reviewing that PR. 

Reviewers must know what the fundamental problem is before they can understand if it’s been resolved. Don’t force a reviewer to read through 600 lines of code. Instead, provide a descriptive PR title and high-level summary at the top of the PR. 

This is why it's good practice to create and enforce the use of PR templates, such as: 

• Problem 
• Solution 
• Impact
• Testing plan

Using a template gives reviewers context so they can give meaningful feedback without having to dig through docs or code. It can also help reviewers catch bugs. For example, if the description says the feature flag is always turned off, they can then look for the feature flag to confirm that.

When just starting out with PR templates, start with a really short template because being concise helps the reviewer, which leads to better results. This Axolo blog post includes a good example of a short template. 

Keep PRs short (same with files and functions) 

There is a vicious cycle that happens with long PRs. Long PRs yield long review times, and longer review times yield longer PRs because engineers try to get more code reviewed per PR, repeating the cycle.

It’s actually better to write short PRs. Although initial reviews may still be slow, review times will pick up speed because shorter PRs are easier to review.

It takes discipline to write short PRs. While you may intend to write a short PR, PRs lengthen after tests are written and edge cases are fixed. Sometimes they get longer, too, when code related to a previous PR is refactored in the current PR.

And even if you’re committed to short PRs, that begs the question: How long is too long? 

Some teams prefer to set numeric guidelines such as under 400 lines or under 10 files. Other teams prefer to break PRs down into logical units like event publisher, controller, or database layer rather than setting numeric guidelines. It’s most important to pick a method and decide how to enforce it; PRs won’t just magically get shorter because the team agrees they need to.

Why are shorter PRs better for development?

• Shorter PRs lead to fewer bugs and more comments per line. It’s much easier to catch bugs in a shorter PR, while an extremely long PR is more likely to get a rubber-stamp “LGTM” (Looks Good To Me). 
• Reviews are faster. Engineers can review a short 300-line PR during a gap between meetings. It can be harder to carve out the time needed to review a 600-line PR.

Manage PR disagreements through direct communication 

As a new grad engineer, it’s hard for me to know what to do when two senior engineers disagree about my PR. There also can be a long time between responses in the GitHub comment section. 

Instead of waiting for the back and forth, try initiating direct communication through Zoom or Slack as soon as the conversation starts. Update the GitHub comment afterward to close the loop on what was decided and why.

Why does direct communication help?

• Feedback will be faster.
• Verbal communication leads to fewer misunderstandings.
• 10 minutes of verbal communication is a much more thorough review than a few back and forth GitHub comments.
• Direct conversation prevents constant context switching between other work and checking the PR comment section.

Avoid rewrites by getting feedback early

As a new grad, I sometimes struggle with when I should ask for help. While some questions are better to answer through research or trial and error, there can be a heavy price to pay for going in the wrong direction for too long. Early feedback tends to be more useful. 

To decide whether it’s time to ask for help, try these steps:

1. Figure out the urgency — if it’s urgent, ask for feedback right away.
2. Before searching for a solution, make sure you’re asking the right question.
   1. It can be difficult to find the answer to a very specific question, but much easier to answer a broader version of that question.
   2. For example, a specific question might be: “What’s the plural of PR (pull request)?” But an answer may be more readily available for the general question: “How do plural acronyms work?”    
3. Search Google, the codebase, internal documentation, and/or Slack
4. Try rubber duck debugging (https://en.wikipedia.org/wiki/Rubber_duck_debugging)
   1. Although traditional “rubber duck debugging” involves talking out loud to an inanimate object, it can be even more helpful to write the problem down.
   2. Begin by summarizing the problem and what you’ve tried so far, including any potential solutions.
5. Ask for help
   1. Tell people how long you’ve been stuck, what you’ve tried, and what you’re thinking.
   2. If it’s urgent, explain the urgency. If it’s a one-off request and not a pattern of behavior — and even if you should have asked earlier — people tend to be sympathetic to: “Hey, I’m sorry, I know I should’ve asked earlier, but I could really use your help because <reason>.” .

One way to get early feedback is to create a PR in “draft mode,” which prevents it from being merged. Developers usually don’t request review before their code is working, but creating a draft PR for a code skeleton or even slightly buggy code can help solicit helpful feedback.

Early feedback allows early course correction, which saves time re-writing features and tests if something is wrong. 

Request additional reviewers to create dialogue

Sometimes we want our code merged in quickly, so we seek an approval instead of a proper review and dialogue. However, shallow reviews defeat the purpose of code review.

One step that can help open up a dialogue is to request more reviewers. 

Why does requesting more reviewers help?

• It adds more perspectives. If only one approval is required, and only one review requested, other teammates will never get a chance to disagree.
• People are available at different times. Requesting more reviewers increases the likelihood that eyes get on the PR sooner.
• If there’s a more general coding style/design disagreement in the PR, others have a chance to weigh in or at least become aware of it.
• Standups are by nature more status updates than working in the weeds. When someone is requested on a PR, they can read exactly what a teammate is working on and can reference that code if they encounter a similar issue.

Conclusion

Writing easy-to-review PRs and maintaining a robust reviewing culture is key to good software engineering. PRs aren’t just for getting code approved; they’re meant to foster feedback and resolve disagreements.

Whether you’re an intern learning how the industry works or a senior engineer trying to set a good example for your team, creating clean PRs and managing feedback well will accelerate your process. Building this habit will pay off in the long run and ensure good practices are being followed even when times are busy. Your future self and future colleagues will thank you.