How to prepare a good pull request

Published on ; updated on

  1. A pull request should have a specific goal and have a descriptive title. Do not put multiple unrelated changes in a single pull request.

  2. Do not include any changes that are irrelevant to the goal of the pull request.

    This includes refactoring or reformatting unrelated code and changing or adding auxiliary files (.gitignore, .travis.yml etc.) in a way that is not related to your main changes.

  3. Make logical, not historical commits.

    Before you submit your work for review, you should rebase your branch (git rebase -i) and regroup your changes into logical commits.

    Logical commits achieve different parts of the pull request goal. Each commit should have a descriptive commit message. Logical commits within a single pull request rarely overlap in the lines of code they touch.

    If you want to amend your pull request, rewrite the branch and force-push it instead of adding new (historical) commits or creating a new pull request.

  4. Make clean commits. Run git diff --check origin/master, and if it reports any issues like trailing whitespace or missing newlines at the end of the file, fix them.

.gitignore

My .gitignore policy is that the project-specific .gitignore file should only contain patterns specific for this project. For instance, if a test suite generates files *.out, this pattern belongs to the project’s .gitignore.

If a pattern is standard across a wide range of projects (e.g. *.o, or .stack-work for Haskell projects), then it belongs to the user-specific ~/.gitignore.

stack.yaml

(This section is specific to Haskell.)

My policy is to track stack.yaml inside the repo for applications, but not for libraries.

The rationale is that for an application, stack.yaml provides a useful bit of metainformation: which snapshot the app is guaranteed to build with. Additionally, non-programmers (or non-Haskell programmers) may want to install the application, and the presence of stack.yaml makes it easy for them.

These benefits do not apply to libraries. And the cost of including stack.yaml is:

UPDATE (2017-06-01): I now include stack.yaml in some of my library repos. The reason is that it is needed for stack’s recommended travis configuration.