Skip to main content

Crafting a Good Pull Request

A guide to crafting good pull requests.

Atomicity​

An atomic pull request is self-contained and independent of other pull requests. It should be a small, production-ready change with everything included. In practice, an atomic pull request will contain all supporting tests, documentation and relevant examples. In a sense, it is similar to an atomic operation in concurrent programming.

Pull requests that merge into master should be atomic.

Size Matters​

Each pull request should be small and address exactly one concern. As a rule of thumb, a pull request should contain ~10 files with substantial changes. Large pull requests are difficult and tedious to review. For an in-depth discussion, see Google's Engineering Practices Documentation.

The Anatomy of Good Pull Requests​

A good pull request describes what changes are made and why they are made.

Short Titles​

The title of a pull request should be a short summary of what is being accomplished. It should be informative enough to describe what the pull request does without reading the description.

A title should be phrased as an imperative sentence (without a trailing .). For example, "Remove Palette and replace it with Styles", instead of "Removing Palette and replacing it with Styles".

Following the naming convention outlined in Git Gud, a branch name is not a suitable title. Using it will result in something ill-formatted and vague. Since the default title generated by GitHub is derived from the branch's name, it should therefore be avoided.

Detailed Descriptions​

The title should be a short summary while the description should contain the details and information needed for a reader to understand the changes holistically, i.e. (why something was changed). It should not be empty barring the most trivial bug fixes. Bullet points may be preferred over a lengthy prose in some situations.

If a pull request contains changes to the UI, consider attaching images of the altered UI.

Listed below are some useful prompts for crafting descriptions under different circumstances. They should not be blindly applied to all descriptions.

  • Are there other pull requests or issues related to this pull request?

  • What is the problem being solved?

  • Why is this the best approach?

  • Are there any shortcomings to this approach?

  • Is there any relevant background information or context?

  • Is there any future work that needs to be addressed?

Counterexamples​

Listed below are some examples of inadequate descriptions. They lack information on what is being changed and why so.

  • "Fix build"

  • "Fix bug"

  • "Add function"

  • "Delete class"