The anatomy of a perfect pull request

Hugo Dias
Hugo Dias
May 31, 2018 · 4 min read
Photo by Ash Edmonds on Unsplash

Writing clean code is just one of the many factors to care about when creating a pull request.

Large pull requests cause a big overhead during the code review and facilitate bugs introduction in the codebase.

That's why you need to care about the pull request itself. It should be short, must have a good title and description and should do just one thing.

Why should I care?

  • A good pull request will be reviewed quickly;
  • It reduces bug introduction into codebase;
  • It facilitates new developers onboarding;
  • It does not block other developers;
  • It speeds up the code review process and consequently the product development.

The size of the pull request

Classic, right?

The first step to identify problematic pull requests is to look out for big diffs.

There are several studies showing that its harder to find bugs when reviewing a lot of code.

In addition, large pull requests will block other developers who may be depending on the code.

But how can we determine the perfect pull request size?

A study of a Cisco Systems programming team revealed that a review of 200–400 LOC over 60 to 90 minutes should yield 70–90% defect discovery . Source



Source: small business programming

As we can see from the chart above, pull requests with more than 250 lines of changes tend to take more than 1 hour to be reviewed.


Break down large pull requests into smaller ones

Feature breakdown is an art. The more you do it, the easier it gets.

What do I mean by Feature breakdown?

Learning by doing

Let’s say that you need to create a subscribe feature on your app. It's just a form that accepts an email address and saves it.

Without knowing how your app works, I can already break it in 8 pull requests.

  • Create a model to save emails
  • Create a route to receive requests
  • Create a controller
  • Create a service to save it in the database (business logic)
  • Create a policy to handle access control
  • Create a subscribe component (frontend)
  • Create a button to call the subscribe component
  • Add the subscribe button in the interface

As you can see, we broke this feature into many parts and most of this tasks can be done simultaneously by different developers.


Single responsibility principle

Source: https://www.toptal.com/software/single-responsibility-principle

The single responsibility principle is a computer programming principle that states that every module or class should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by the class.

Just like classes and modules, pull requests should do only one thing.

Before submitting a PR for review, try applying the principle of single responsibility. If this code is doing more than one thing, break it into other Pull Requests.


Title and description matter

When creating the PR you should care about the title and the description.

Imagine that the code reviewer is arriving in your team today without knowing what is going on, and even so, they should be able to understand the changes.

What a good title and description looks like

The title of the PR should be self-explanatory

The title should be sufficient to understand what is being changed.

Some examples:

Make a useful description

  • Describe what was changed in the pull request.
  • Explain why this PR exists.
  • Make it clear how it does what it sets out to do. E.g: Does it change a column in the database? How is this being done? What happens to the old data?
  • Use screenshots to demonstrate what has changed.

Recap

Pull request size

It should be small. The pull request must have a maximum of 250 lines of change.

Feature breaking

Whenever its possible break pull requests into smaller ones.

Single Responsibility Principle

The pull request should do only 1 thing.

Title

Make a self-explanatory title describing what the pull request does.

Description

Detail with what was changed, why it was changed, and how it was changed.


Thanks!

Here is my recent post on medium: Extracting a React JS component and publishing it on NPM

Welcome to a place where words matter. On Medium, smart voices and original ideas take center stage - with no ads in sight. Watch
Follow all the topics you care about, and we’ll deliver the best stories for you to your homepage and inbox. Explore
Get unlimited access to the best stories on Medium — and support writers while you’re at it. Just $5/month. Upgrade