How to prepare a good pull request
Published on ; updated on
A pull request should have a specific goal and have a descriptive title. Do not put multiple unrelated changes in a single pull request.
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.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.
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:
- The snapshot version gets out of date quickly, so you need to update this file regularly.
- This file is often changed temporarily (e.g. to test a specific version of a dependency), and if it is tracked, you need to pay attention not to commit those changes by accident.
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.