What Every Teammate Wants to See in Your Next Pull Request

Everyone keeps telling us to be gentle when reviewing Pull Requests, and that’s the right thing to do, but are we nice with who reviews ours?

A Pull Request is a way to communicate with other individuals who are responsible for the project’s maintenance, so we should do a good job improving our Pull Requests code and documentation to make their work straightforward. It’s also in our best interest to do that since the easier the review is the faster we’ll get our code merged.

Who reads our Pull Requests?

Creating a reasonable Pull Request is a matter of walking in the shoes of people that will open it later, who fall into the following categories:

• Code Owners: who are in charge of reviewing and merging the code.

• Developers implementing new features: who are changing previously merged code and need to understand why that code is there.

• Developers fixing bugs: who encounter a bug and need to go deep in the git history to understand where the regression has been introduced and why.

We should be respectful of each of them while creating a Pull Request since facilitating their work both in the present and in the future is what makes software development honest and practical. “Time is running out” kind of excuses here are not valid, every moment that we don’t spend improving Pull Requests that we leave in our project is a lot of time wasted for who will interact with them later.

A Pull Request is created one time but evaluated multiple times.

Do the math!

What reviewers dream to see

Let’s take a look together at what makes a Pull Request easy to review.

Well organized code

Code is the soul of the Pull Request, but it won’t be my focus here. You can find a lot of resources about this on the Internet. I’ll only mention some general rules to apply code side:

• Make it small, try to break it into multiple Pull Requests.

• Write well-separated and well-documented commits, the why is the most important thing.

• Add tests, they’ll help reviewers understanding the context of the Pull Request.

• Take a look at the contribution guidelines usually at CONTRIBUTING.md; it may have important information.

Meaningful title

It needs to clearly and exactly describe what has changed. For example:

🔴 Bad: This one is too generic and doesn’t provide any value.

✅ Good: This one is specific and explain what changed.

If you can only come up with a generic title, that’s probably a first signal that we could split the Pull Request into multiple ones.

A rule that I apply to determine if a title is good is to ask myself this:

Could this title fit a CHANGELOG entry?

This soft-rule comes from my experience maintaining Solidus, the open-source project I mostly work on. On Solidus, before each release, we run a script that auto-generates entries for the CHANGELOG.md file using Pull Request’s titles. When they are self-explanatory, there’s much less manual work to do for maintainers.

A description that really helps

Here’s the core of the Pull Request documentation. It’s quite common to assume that the Pull Request title and commits are enough for people to understand the context, but that’s not always true. For example, let’s consider this Pull Request title:

I’m sure that when reading the Pull Request title today, with all the team focused on this specific change, everyone will immediately get what’s going on and it could be tempting to leave the Pull Request’s description empty. But what happens if you have to come back to this Pull Request in 1 year? Will you immediately remember how this switch has been done, and why?

Luckily, the “how” could still be read by commits description, even if it would be more time and energy-consuming to scan all the commits to get the sense of what’s happening. The real problem is that it’s tough to see the “why” of the change.

Let’s see what makes a good Pull Request description:

• It contains a quick recap of the changes introduced. It doesn’t have to be a lot of text, just something to quickly understand what changed without opening every single associated commit.

• It describes why things changed, try not to take for granted that the reader has a clear picture of the state of the system when the change happened. If the Pull Request comes from a change request done via an issues tracker, it could be handy to link that issue here.

• It describes the steps to reproduce the changes introduced with the Pull Request. It could be calling a method from a console, or make some interactions in a UI. It will drastically help reviewers to touch with hands the code they are seeing. It allows them to figure out any doubts that could arise in autonomy, without the need to ask for clarification and extra back and forth with the author.

• It includes screenshots: if the change has an impact on how the website looks or on any other visual part of the system, it’s a good idea to attach some screenshots. It will be super easy for those who look at the Pull Request to identify what changed and how.

With GitHub, you can guide the Pull Request author into providing all these things by using Pull Request templates. It’s a feature that allows to specify a template that will populate the Pull Request description for any new Pull Request. It’s just a matter of adding a markdown file into .github/PULL_REQUEST_TEMPLATE.md. Here’s a collection of ready-to-grab Pull Request templates and the GitHub documentation page for Pull Request Templates.

There’s also another hidden, not well-documented GitHub feature, that allows having multiple templates for Pull Requests. You can get them by just reflecting this filesystem structure:

.github/
  - PULL_REQUEST_TEMPLATE.md .       # The Default Template
  - PULL_REQUEST_TEMPLATE/extra.md   # Additional templates

When you create the Pull Request, you can use the infrastructure template by adding the &template=extra.md in the URL’s query params.

The right reviewers

The policy about asking specific persons to review our code could change depending on the project. For this reason, GitHub offers a Code Owners functionality that allows us to specify rules that automatically trigger review-requests from the right people. Here’s an example of a CODEOWNERS file:

*                   @project/backend-team
/app/javascript/    @project/frontend-team
/docs/              @project/documentation-team

This will ask for a review to the project/backend-team for all changes in the project excluding those that are located in app/javascript and docs, which will ask for a review to their respective teams.

I suggest integrating the usage of this feature with a clear team policy on how much time and how often developers are expected to review others’ code.

Preview App linked to each Pull Request

Reviewers’ work is way smoother when they can test what a Pull Request changes before merging, and it’s a good practice to have a preview environment, very similar to staging, generated for each Pull Request open.

If you are deploying on Heroku, you can just use their Review Apps, whose setup is quite simple. It’s also possible to build something very similar by yourself: GitHub offers a Deployments API to create a Deployment section in the Pull Request page. By calling something like:

curl -X POST -H "Content-Type: application/json" \
-H "Accept: application/vnd.github.ant-man-preview+json" \
"https://api.github.com/repos/nebulab/project/deployments/42/statuses?access_token=XXX" \
-d '{ "state": "success", "environment_url": "https://your-custom-url/", "environment": "preview"'

You’ll see the deployment link in your Pull Request.

At Nebulab, we use CircleCI and we’ve created a CircleCI Orb that allows us to have this functionality ready in minutes in all our projects by adding the following lines to the .circleci/config.yml:

version: 2.1

orbs:
  feature-branch: nebulab/feature-branch-preview

workflows:
  cool-workflow:
    jobs:
    - do-cool-things
    - run-specs
    - feature-branch/preview:
        domain: <domain-for-preview-apps>
        github_repo: <your-github-repository>
        github_token: <your-gh-auth-token>
        letsencrypt_email: <email-for-ssl-cert>
        server: <docker-server-to-run-containers>
        user: <docker-server-user>

I bet you now want to be nice too!

After reading this post, you should! Being nice with reviewers gives you better and contextualized quality checks for your code, which should help you grow as a developer. It decreases time-to-merge for your code and will increases the velocity to implement new features that depend on existing ones. Debugging is also faster since you don’t have to read all the code to understand how and why a specific thing has been done. Helping reviewers means helping your team and your future self!