How to prepare a good pull request
September 18, 2016
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 (
.travis.ymletc.) 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 showon your commits. It will show you issues like trailing whitespace or missing newlines at the end of the file.
.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
If a pattern is standard across a wide range of projects (e.g.
.stack-work for Haskell projects), then it belongs to the user-specific
(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
- 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 my library repos. The reason is that it is needed for stack’s recommended travis configuration.