Anatomy of a perfect pull request

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

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

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

Why should you 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, product development

The size of the pull request

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

Several studies show that it is 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.

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.

With this number in mind, a good pull request should not have more than 250 lines of code changed.

^{Image from small business programming.}

As shown in the chart above, pull requests with more than 250 lines of changes usually take more than one hour to review.

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?

Feature breakdown is understanding a big feature and breaking it into small pieces that make sense and that can be merged into the codebase piece by piece without breaking anything.

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 into eight 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, I broke this feature into many parts, most of which can be done simultaneously by different developers.

Single responsibility principle

The single responsibility principle (SRP) is a computer programming principle that states that every module or class should have responsibility for 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.

Following the SRP reduces the overhead caused by revising a code that attempts to solve several problems.

Before submitting a PR for review, try applying the single responsibility principle. If the code does more than one thing, break it into other pull requests.

Title and description matter

When creating a pull request, you should care about the title and the description.

Imagine that the code reviewer is joining your team today without knowing what is going on. He should be able to understand the changes.

Image by:

^{What a good title and description look like}

The image above shows what a good title and description look like.

The title of the pull request should be self-explanatory

The title should make clear what is being changed.

Here are some examples:

• Add test case for getEventTarget
• Improve cryptic error message when creating a component starting with a lowercase letter

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— for example, does it change a column in the database? How is this done? What happens to the old data?
• Use screenshots to demonstrate what has changed.

Recap

Pull request size

The pull request must have a maximum of 250 lines of change.

Feature breakdown

Whenever possible, break pull requests into smaller ones.

Single Responsibility Principle

The pull request should do only one thing.

Title

Create a self-explanatory title that describes what the pull request does.

Description

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

This article was originally posted at Medium. Reposted with permission.