3 reasons why nobody wants to review your pull requests

Photo by Yancy Min / Unsplash

I have accumulated over 11 years of experience working as a Software Engineer. Throughout my career, I have had the opportunity to work in various companies located in both Asia and Europe. One aspect of my job that has consistently posed challenges is the review of pull requests. From my very first job, I quickly realized that reviewing pull requests can be a daunting and time-consuming task. It requires not only making proper pull requests, but also meticulously examining and providing feedback on the pull requests submitted by my colleagues. Despite its inherent difficulties, I have come to appreciate the importance of thorough pull request reviews in maintaining code quality and fostering collaboration within a development team.

💡
Why do you think the “in review” column of issues in boards is almost always the longest?

One of the main challenges that arises in the software development process is the difficulty of conducting effective PR reviews. These reviews, while essential for ensuring code quality and collaboration, can often be perceived as tedious and unengaging by fellow developers who are also working on the same project. It is important to find ways to make the PR review process more stimulating and interactive, in order to maintain the interest and involvement of all team members throughout the development cycle.

Let’s explore together three big mistakes that will make your PR stay a long time in an idle state.

1. Enormous diffs

There is nothing less appealing for a reviewer than a pull request (PR) with a difference of several thousand lines of code (LOC) across multiple files. It can be quite overwhelming for the reviewer to go through such a large diff, and it may deter them from conducting a thorough review of the feature.

Who wants to spend their time reviewing a diff that spans a thousand lines of code? When the diff is too long, it becomes more likely that other developers will simply leave a quick “LGTM” (Looks Good To Me) comment without taking the time to carefully review the changes. This can result in missed opportunities for identifying potential issues or improvements in the code.

It is important to ensure that pull requests are manageable in size, making it easier for reviewers to provide meaningful feedback. By keeping the changes concise and focused, it encourages reviewers to engage in a more detailed and comprehensive review process, which ultimately leads to higher code quality and better collaboration within the development team.

LGTM (Looks Good To Me) ❤️

The larger the difference in code changes, the more likely the pull request (PR) will have bugs, be time-consuming to review, and potentially cause delays. A big PR demands significant attention and may even require additional testing from the reviewer(s).

On the other hand, if the code changes are kept smaller and more focused, the pull request will be easier to review and less likely to introduce bugs or delays. By breaking down the changes into smaller, manageable chunks, the reviewer can provide a more thorough and efficient review process. This approach also allows for better collaboration and reduces the risk of conflicts or misunderstandings.

Therefore, it is highly recommended to aim for smaller and more focused pull requests. This will not only expedite the review process but also ensure a higher quality of code and a smoother workflow for everyone involved.

Graph from Rodrigo Miguel
“We can view the transaction cost as the cost of sending a batch to the next process and the holding cost as the cost of not sending it.” - Don Reinertsen

In the figure above, it is clear that the optimal PR size is around 450 LOC. Considering various studies and my own perspective, I strongly believe that in order to ensure timely code reviews of 1 hour or less, it is highly recommended to keep the PR size below 400 LOC. However, it's important to note that this suggested number should not be treated as a strict rule, as there may be cases where refactoring or boilerplate PRs could extend up to this limit without being overly complex or convoluted.

2. Generic title and bad description

The title of a pull request plays a vital role in effectively communicating its purpose. It is significant to ensure that the title is not only clear and concise, but also provides enough context to understand the intention behind the pull request. By including relevant information in the title, such as a reference to the related issue in your project management software, you can provide additional clarity to the reviewers. This will help them better understand the changes being proposed and expedite the review process. Remember, a well-crafted pull request title sets the stage for a productive collaboration and ensures that everyone involved is on the same page.

For example, a simple and efficient PR title:

Fix: Clean config file location cache only when needed #5007

An example of a bad commit title would be:

Update code

A vague and generic commit title like this does not provide any useful information about the changes made in the commit.

The description of the PR should also provide further information on the changes and the different commits linked. It should describe what changed and why, but not how.

💡
On Github, you can create a file named pull_request_template.md so that any new PR created will use this template. See Github documentation

A template of a detailed PR description could be like this:

## Description
<!--- The WHAT -->
<!--- Describe your changes in details -->

## Motivation and Context
<!--- The WHY -->
<!--- Why is this change required? What problem does it solve? -->

## Related Issues
<!--- List of related issues -->

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->

## Screenshots (if appropriate):

Benefits of Having Good Pull Request Titles and Descriptions

Having clear and informative pull request titles and descriptions brings several benefits to the development process:

  1. Improved Understanding: A well-crafted title provides context and sets expectations for reviewers, helping them understand the purpose and scope of the pull request. A comprehensive description further clarifies the changes made and the motivation behind them, ensuring that everyone involved is on the same page.
  2. Efficient Review Process: When reviewers can quickly grasp the essence of a pull request from its title and description, they can focus their attention on providing meaningful feedback and identifying potential issues. This streamlines the review process, saving time for both the reviewer and the pull request author.
  3. Effective Collaboration: Clear titles and descriptions foster effective collaboration within the development team. By providing relevant information and linking related issues, the pull request becomes a centralized source of knowledge, enabling better communication and coordination among team members.
  4. Reduced Misunderstandings: With well-defined titles and descriptions, there is less room for misunderstandings and confusion. Reviewers can better understand the changes being proposed and provide more accurate feedback. This reduces the likelihood of misinterpretation and ensures that the pull request aligns with the project's goals and standards.
  5. Maintained Code Quality: A pull request with a clear title and description encourages thorough reviews and attention to detail. Reviewers can dive into the code changes more confidently, identifying potential bugs, suggesting improvements, and maintaining the overall code quality.

In summary, investing time and effort in creating good pull request titles and descriptions pays off by facilitating smoother and more efficient collaboration, improving code quality, and fostering a positive development environment.

3. Multiple purposes

I’m sure you have encountered a situation where you came across a piece of code that required fixing or an algorithm that needed improvement. In such instances, you might have thought to yourself:

Oh! I’ll simply rectify it and submit a pull request with the necessary changes.

Well, you’re wrong!

When creating a pull request (PR), it is important to ensure that each PR focuses on a single purpose and responsibility. It is crucial to avoid including any unrelated code modifications in the PR, regardless of whether they are in a separate commit or not. If there are multiple changes that need to be addressed, it is recommended to divide the PR into multiple ones, each addressing a specific change. By following this approach, it allows for better clarity and organization in the PR process, making it easier for reviewers to understand and provide feedback on the changes made.

When creating a pull request, it is crucial to ensure that each PR focuses on a single purpose and responsibility. Including unrelated code modifications in a PR, even if they are in separate commits, can lead to confusion and inefficiency in the review process. By dividing the PR into multiple ones, each addressing a specific change, it allows for better clarity and organization. This approach makes it easier for reviewers to understand and provide feedback on the changes made, leading to a smoother and more effective review process. Additionally, separating changes into distinct PRs helps maintain code quality, reduces the risk of introducing bugs or conflicts, and promotes better collaboration within the development team.

A few more tips

  • Before submitting your PR, make sure to rebase the latest version of the main branch onto your PR branch.
  • If your PR is not yet ready to be reviewed, you can use the keywords [WIP] or [DRAFT] in the title.
  • If there is something important to notify the reviewer(s), you can add inline comments.
  • Apply your code formatting standard to your changes.
  • Use a semantic-based commit message format, such as the one used by Angular.
  • For front-end related PRs, including a screenshot can be more helpful than explaining with many lines of text.

Wrapping up

Let's expand on the 3 main rules for your "ideal" pull request:

  1. Keep it reasonably small and comprehensive. It's important to ensure that your pull request is of a manageable size and covers all the necessary aspects. By keeping it concise yet thorough, you'll make it easier for reviewers to understand and provide feedback.
  2. Provide an explanatory title and a helpful description. When creating a pull request, it's crucial to give it a clear and descriptive title that summarizes the purpose of the changes. Additionally, a well-written description can provide context, explain any challenges faced, and highlight the benefits of the proposed changes.
  3. Focus on one and only one subject, whether it's a feature, a bug fix, a refactoring, documentation, or any other specific area. By honing in on a single subject, you can ensure that your pull request remains focused and targeted. This allows for better collaboration and makes it easier to track and review the changes made.

Remember, following these guidelines will not only improve the quality of your pull requests but also streamline the reviewing process and increase the chances of your changes being merged smoothly.