We've posted about best practices for git, but let's focus on creating Pull Requests that are easy to understand and review.
Following these recommendations will accelerate the process of getting your PR approved and merged - and your reviewers will love you for it! You'll also find it easier to understand changes over time as you walk through git history.
1. Keep it Small
Minimize the size and scope of the Pull Request. The smaller it is, the easier it is to review and get it approved and merged. Generally, each story or ticket in your project management system should have a single PR - so keeping the size of your PRs small is tightly coupled with writing good stories.
2. Review Your Own Pull Request
Review your own PR before you tag any other reviewers! This ensures that you'll always see the PR from a reviewer's perspective. I frequently catch issues that I wouldn't otherwise notice if I just put up a PR without reviewing it first.
3. Use a Pull Request Template
An awesome commit message contains these elements:
- A concise description of what's being changed and why.
- A link to the relevant ticket(s) on Trello, Jira, Pivotal Tracker, etc. Use full URLs rather than issue numbers.
- Gotchas! Any important notes that reviewers should pay attention to.
- Frontend pull requests should include screenshots and/or animated GIFs demonstrating new behaviors. We like GIPHY Capture.
Creating a Pull Request template on GitHub, Bitbucket, and GitLab is easy.
# DESCRIPTION
Briefly describe what is being changed, and why.
# LINKS
[Jira Ticket](https://jira.atlassian.com/...)
[Design Document](https://docs.google.com/...)
# DATA MIGRATIONS
Describe any schema changes or data migrations.
# GOTCHAS
What should reviewers keep an eye out for?
# SCREENSHOTS & ANIMATIONS4. Minimize the Number of Commits!
Once development is complete and you're ready for your code to be reviewed, Pull Requests should generally be squashed into a single commit.
If there's a lot going on, we occasionally divide a PR into a few commits when it organizes changes into discrete collections that are easier for reviewers to work through.
What we really avoid is "train of thought" commits that represent the natural progress of development.
5. Adhere to Linting / Style Guide
All code committed should adhere to the style guide and linting rules that have been configured for the repository. We commit our linting configuration (.eslintrc.yml , .prettierrc, .rubocop.yml, etc) to our repositories so that everyone is using the same rules.
6. No Stylistic / Linting Changes Should Exist Alongside Functional Changes
As a reviewer, it's cognitively expensive to review a PR that contains both stylistic AND functional changes. Don't do it!
When code style or linting rules change, use a single PR to make those changes. It should not contain ANY functional changes.
Any PR that contains functional changes should not contain any purely stylistic changes.
7. Include Test Coverage
Any pull request that adds or changes functionality must include test coverage which ensures that the changes behave as expected. Test coverage should only ever increase!
