Building a Great Pull Request

Feb 2, 2018

I love pair-programming with other developers, in part because I always end up learning new ways to improve how I work. One aspect of my workflow that has benefitted from pairing is writing pull requests (or "PRs"). A clear, well-built PR is an essential part of a collaborative workflow, and through pairing, I've picked up several techniques that have made my PRs a little bit better and easier to create.

Contributing to open source? For a great walkthrough on how to create a pull request for an open source project on GitHub, be sure to check out Matt Stauffer's guide.

TIP 1: Give your feature branch a clear name

A well-formed PR should start with a well-named feature branch. At Tighten, we typically use the Git Flow workflow (similar to GitHub Flow). This means we use a main branch for "production" that is only merged into from develop. Developers create branches off of develop for their work on an individual feature, and giving that feature branch a clear name is an important first step when collaborating with others on a single repository.

I picked this tip up from Keith Damiani: if your project has more than two contributors, it can be useful to prefix your branch names with your initials. For example, I prefix lh- to all my feature branch names: git checkout -b lh-building-a-great-pull-request. This also allows you to filter down to your personal branches with git branch -v | grep lh- when there are a lot of active branches in play. It also makes naming branches a bit less daunting, because every branch you create is automatically scoped to you!

TIP 2: Give your commits and PRs active and descriptive titles

For both commit messages and PR titles, it's a good idea to use the present imperative tense — for example, use “Fix dashboard typo” instead of “Fixed” or “Fixes.”

There's a great blog post by Chris Beams entitled "How to Write a Git Commit Message" that's very much worth reading, but one of the most interesting points is his take on the imperative (5. Use the imperative mood in the subject line). One strong reason for using the imperative? Because system-generated commits from Git itself (like "Merge branch...") are written that way.

Also, if you've created a PR that's not quite ready to be merged, give the title a prefix like “WIP:” or “IN PROGRESS:” to avoid someone accidentally reviewing your work prematurely.

TIP 3: Streamline your process of creating a new PR

If you are a GitHub user, our own Jose Soto made a little bash function in order to streamline the process of opening a new PR:

function gpr() {
  git push origin HEAD
 
  if [ $? -eq 0 ]; then
    github_url=`git remote -v | awk '/fetch/{print $2}' | sed -Ee 's#(git@|git://)#http://#' -e 's@com:@com/@' -e 's%\.git$%%'`;
    branch_name=`git symbolic-ref HEAD 2>/dev/null | cut -d"/" -f 3`;
    pr_url=$github_url"/compare/main..."$branch_name
    open $pr_url;
  else
    echo 'failed to push commits and open a pull request.';
  fi
}

With this, typing gpr in your Terminal will open a fresh "Create Pull Request" screen on GitHub, ready to review!

If you are a hub user, you can create a PR easily, and copy the link to your clipboard, using hub pull-request -m "message" | pbcopy.

TIP 4: Give your PR a meaningful description

A PR description section is where you let the reviewer know why you opened the pull request; the more information you give them before they look at your actual code, the easier their job will be. A basic description usually has these elements:

"Why?" Why is this new code necessary? Providing a little extra context helps give your reviewer a clearer understanding of what they are about to be looking at.
"How?" Provide a bullet point list of the most important commits, expanding on each as necessary.

I use the alias: alias desc="git log --no-merges --pretty=format:'- %s' main.. | pbcopy" to quickly grab a list of commits that haven't yet been merged into develop. The alias copies the commits to my clipboard; from there, I paste the text into the "How?" section and edit it down as needed.

"What?" Demonstrate the functionality that was added or changed, calling out particular parts of your feature that warrant extra attention. Images and animated GIFs are a great way to do this.

GIF showing example "why", "how", "what" in a pull request

TIP 5: Show your functionality visually, whenever possible

You can save your reviewer a lot of time by allowing them to get the gist (gist--get it?) of your changes without pulling your branch down to their machine and clicking through a UI.

You can take screenshots natively on OSX and save them into your clipboard directly by doing Command-Control-Shift-4. You can then just paste these screenshots directly into your PR description on GitHub!
Sometimes you want to share an entire screen that extends "below the fold." You can capture everything on a web page using Chrome by going to the inspector, opening the device toolbar (phone/tablet view), and then clicking the three dots at the top right and using Capture screenshot or Capture full-size screenshot.

GIF showing how to use Chrome to capture screenshots

When it comes to capturing animated GIFs, licecap is a popular option. I have also started using Giphy Capture, and it is a beautiful experience.

Here is an example of listing the highlights of your commits at a glance, and giving a preview of the visual aspects of a PR:

Image showing an example PR

TIP 6: Review your own PR before you assign it to others

Make sure you are caught up with the latest changes in the develop branch; resolve any conflicts that might exist, so that merging is as easy as pressing a button.
Run your tests! Nothing is worse than pushing up a PR that you think is done, but which actually breaks functionality.
Lint your code! Use your team's established standard tools; at Tighten, we use PHP Coding Standards Fixer, scss-lint, tlint, and prettier for most projects.
Make sure you sanity-check your changes. After pushing up your code, you can use the awesome GitHub UI to read through all of your changes, and catch any glaring issues.