The anatomy of a perfect pull request
How does a good pull request look like
Writing clean code is just one of the many factors to care about when creating a pull request.
Large pull requests cause a significant 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, have a good title and description, and 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
The first step to identify complex pull requests is to look out for big diffs.
Several studies are showing that it's 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
With this number in mind, a good pull request should not have more than 250 lines of code changed
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?
Is understanding a big feature and breaking it into small pieces that make sense and 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, we broke this feature into many parts, and most of these tasks can be done simultaneously by different developers.
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.
Pull requests that follow 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 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.
The title of the PR should be self-explanatory
The title should be sufficient to understand what is being changed.
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. 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 possible, break pull-requests into smaller ones.
Single Responsibility Principle
The pull request should do only one 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
