How to create and review a GitHub pull request

by Michael Ernst

March, 2017
Last updated: March 20, 2024

The main way of contributing to an open-source project that is hosted on GitHub is via a pull request. A pull request says, “Here are some changes that I have made in my copy. Please incorporate them into the main version of the project.”

Contents:

(Also see Version control concepts and best practices.)

Preliminaries (setup)

Start on a unit of work

When you are ready to start on a unit of work, such as fixing a bug or implementing a feature, create a branch. A branch is a parallel thread of development — you can create as many branches as you want in your repository, which is like having multiple independent repositories. All the work on every branch is saved on GitHub.

Each working copy has a single current branch. When you commit changes (with git commit) or push commits to GitHub (with git push), they are saved to the current branch. You must always remember which branch you are working on; making changes to the wrong branch is a common mistake.

Here are two ways to change which branch is the current branch. (Before changing the current branch, update your repository from upstream and commit any changes.)

Each branch should represent a logical unit of work. If you are doing two different tasks like fixing a bug and performing a refactoring (or if while doing a task you discover a second, distinct task, like the need to refactor or to improve unrelated documentation), then create two different branches for them. This is a bit of a hassle for you, but it makes reviewing your changes much easier, and the maintainers will be more likely to accept your changes.

Why to work on a branch

Do not work on the main branch in your fork. (It is often named main or master.) If you do so, future pull requests will be cluttered by unnecessary merge commits. The rest of this section explains why; you can skip it unless you want to learn more details.

When a developer merges your work into the main repository, that usually creates a new commit (it contains the same code changes, but has a different identity than the one or more commits that you made in your branch). Whenever a branch isn't identical to upstream, pulling from upstream will create a new merge commit. Once a branch is different from upstream, each pull will accumulate more changes (differing commits) from upstream.

Therefore, it is better to keep your main branch identical to upstream, and create a new branch for each pull request. You will delete the branch when your pull request is merged into the upstream repository. (GitHub will do the deletion automatically if you enable “Automatically delete head branches” in the repository settings of your fork.)

If you do create a pull request from your fork's main branch, leading to spurious merge commits on your fork's main branch, then after all your pull requests are merged, you are probably best off deleting your GitHub fork (do this at GitHub.com) and all clones of it, and then re-creating the fork (do this at GitHub.com) and re-cloning. There are other ways to fix the problem, but they use advanced git commands and are more error-prone.

Committing and pushing changes

Before you start to implement your changes, write tests that currently fail but will pass once you have fixed the bug or implemented the feature. Run the tests locally to confirm that they currently fail. (The project's developer documentation will tell you how to do this.) Now, commit the tests and push them. Check that continuous integration has run the project's tests on your fork and that they failed.

Now, do your work, testing locally and committing logical chunks of work as you go. You can push these commits to GitHub by running git push whenever you like. Eventually, you will be done and ready for a code review.

Being done requires at least the following:

Pull upstream into your branch

Periodically pull upstream into your branch; that is, incorporate into your branch any work that other maintainers have done since you created your branch. It's easier to do this frequently than all at once. Here are two ways to do so:

git pull upstream BRANCHNAME
or
git pull https://github.com/OTHERUSER/REPONAME.git

If this command had any effect, then:

Divide your changes into logical units

Oftentimes, when you are working to add a feature, you will also fix a bug, or add documentation, or perform a refactoring. It is great to make these improvements. However, each pull request should be a single, logical unit. If you have made multiple different changes, create a new branch and a separate pull request for each one.

Your repository might start out having only a branch named all-my-changes (the actual name should be more descriptive!). After you make new branches for the logically distinct changes, you might have branches all-my-changes, add-documentation, and refactoring. While you develop, periodically pull the main, add-documentation, and refactoring branches into all-my-changes.

Create pull requests for each branch when it is ready. Don't create a pull request for all-my-changes until the pull requests for ancillary branches have been merged into the main branch, and you have merged the main branch into all-my-changes.

It's a bit more work to separate different changes into different branches, but it makes each pull request much easier to understand. Each pull request can be reviewed more quickly. A change history with more, smaller commits is more helpful to future developers. Test failures are easier to understand. Any interactions between changes are easy to see.

Creating a pull request

Once you are happy with your work and you believe it is ready to be incorporated into the project's main repository, you can create a pull request.

  1. Update your branch (which is in your fork) from upstream.
  2. Ensure that the branch passes all tests both locally and on continuous integration.
  3. On your fork's GitHub page, click on “New pull request”, which appears just above the list of files.
  4. The last dropdown box is by default “compare: main”; change that to your branch. Wait a moment until GitHub shows you the changes, then review all of them (you will need to scroll to see them all).
  5. If there are any changes that are not related to main topic of your branch (or gratuitous whitespace/formatting changes), then don't make the pull request. Instead, go back to your working copy on your computer, undo those changes, and commit. Then start over to make the pull request, with a smaller and cleaner set of changes.
  6. Click “Create pull request”.
  7. Once the pull request passes its tests, assign it to someone, send mail, or write a comment to request a review. The mere creation of the pull request doesn't necessarily signal that you believe your code is ready for review.

Make your code self-explanatory. You should not write pull request comments on lines of code, and you should write very little in the introductory comment to your pull request. Comments in a pull request will never be seen by a programmer reading the source code. If there is information that is needed by a programmer reading the source code, you should put it in a code comment. This also applies to answering questions from reviewers: it is better to clarify the code or add documentation, rather than answering a question in the pull request comment thread.

Creating a private pull request (rare)

Sometimes you want feedback on your code before you are ready to merge it into a different fork. In this case, you can create a pull request between two branches of your fork.

  1. Create a new branch on your fork called (say) “codeReviewTargetTemporary”. It should be up to date with respect to the branch you intend to merge your changes into, which is often the main branch.
  2. Pull the codeReviewTargetTemporary branch into your branch. (Periodically update the codeReviewTargetTemporary from the main branch and pull it into your branch.)
  3. In GitHub, create a pull request that requests to merge your working branch into the codeReviewTargetTemporary branch.
  4. Assign someone to that pull request.
  5. After your code has been reviewed, discard the codeReviewTargetTemporary branch (to which you have made no changes).

Creating a pull request for already-pushed code (rare)

Sometimes, you want a review of code that you have already pushed to GitHub. Or, you want a holistic code review to critique the design of an entire component of your code, rather than incremental code reviews of bits and pieces of it.

GitHub's pull request mechanism does not support this workflow well, but here are two ways to make it work.

Responding to review feedback

  1. You will receive feedback on your pull request. Respond to the feedback by making changes in your working copy, committing them, and pushing them to GitHub when the tests pass locally.

    As soon as you receive feedback, you can start working on it. The reviewer should send you a message and/or assign the code review back to you, but the reviewer might forget, so don't wait for those events.

    Make sure you are working on the right branch; use git branch to check. Never force a push with git push -f. Forcing a push is bad practice, will cause loss of code review comments that GitHub attached to that commit (you can't control which commit GitHub uses), and will cause extra merges or merge conflicts for people who have cloned your branch (such as the people doing the review).

    Go through each piece of feedback.

    When you push commits to GitHub, the pull request will be automatically updated. If you change a line of code on which you received feedback, that feedback is no longer shown by default (or maybe it is shown but marked as out of date). That is, GitHub assumes that if a line near a review comment has been changed, then the review comment has been resolved. This means that you should try not to push changes (such as a change to indentation) that change a line without addressing all the comments related to that line.

  2. After you have addressed all the review feedback, explicitly request a re-review. Do not assume that person will know when you are done. Here are three ways to request a re-review:
  3. There will often be several rounds of feedback and fixes. The reviewer needs to approve your changes, and there may have been parts of your pull request that were confusing or that the reviewer was otherwise unable to review on the first iteration. You are not done until the reviewer has approved your pull request. (Sometimes, multiple people will review your pull request, but it is most efficient to have them do their reviews one at a time rather than concurrently.) Remember to periodically update your branch from upstream.
  4. Eventually, your pull request will be accepted and your changes will be part of the project. Congratulations!
  5. Delete your branch, which no longer serves any purpose. (It's easier to enable “Automatically delete head branches” in the repository settings of your fork. Then you don't have to do this step.)

    Periodically run git remote prune origin to remove deleted branches from your working copy, so that you don't accidentally use them.

Some Git documentation recommends rebasing, amending commits, or other changes to existing version control history. Don't do any of these things. They are confusing and error-prone, they can corrupt your pull request, and they are not necessary. All of your changes will be squashed and merged into a single commit when your pull request is accepted, so don't worry about what the version control history of your branch looks like. Just focus on its differences from the upstream's main branch, which you can see in your pull request.

You will receive email about comments to your pull requests. Don't reply by email. Instead, reply on the GitHub webpage that is referenced by the email. One reason is that if you reply by email, you may needlessly bloat your response with all the quoted text from the email you received. Another reason is that if you reply by email, GitHub may not associate your comment with the right thread in the code review.

Reviewing a pull request

This section is for maintainers who are reviewing and merging a pull request.

This section is currently incomplete, but contains a few tips.

Merging the pull request

Squash, don't just merge

It is desirable to keep the version control history clean: in the main branch, each logical change should be in one commit, even if the pull request entailed work over a period of time and multiple commits on a feature branch. To achieve this, select “Squash and merge” when you merge a pull request. “Squash and merge” results in a single commit that contains all the changes in the pull request. Consistently using “Squash and merge” results in a linear commit history on the main branch.

A single commit is desirable because a pull request represents a single conceptual change that has been tested and reviewed as a logical unit. When a pull request is ready to be merged, it may consist of many commits. Future maintainers will not be interested in each individual commit, such as showing bug fixing within the logical change or interactions during the pull request review. A git history that is littered with lots of little commits is much harder to read and understand.

A side benefit of squash-and-merge is that every commit on the main branch passes tests.

The repository owner can prevent incorrect pull request merges. In the repository settings, in the “Merge button” section, disable “Allow merge commits” and “Allow rebase merging”. You might also want to enable “Automatically delete head branches”.

Commit message

When you squash-and-merge a GitHub pull request, the default first line of the commit message is the pull request's title, and the remainder (which GitHub calls the “extended description”) is the concatenation of the messages for all the commits in the pull request. This latter information is not useful to future developers. Therefore, edit the detail text to remove all the commit messages. Use the pull request's description (the very first comment that was written when the pull request was created), if any.

Another problem with not editing the commit message is that it may leave “[ci skip]” in the commit message, so the merge commit may not be processed by continuous integration such as Azure Pipelines, CircleCI, GitHub Workflows, or Travis CI. (CI may perform some action on every (successful) commit to main.)


Back to Advice compiled by Michael Ernst.

Michael Ernst