See the documented workflow described in the README.md file detailing the sequence of git commands required to submit a PR from a fork of this repo.
PRs should generally address only 1 issue at a time. If you need to fix two bugs, open two separate PRs. This will keep the scope of your pull requests smaller and allow them to be reviewed and merged more quickly.
When possible, fill out as much detail in the pull request template as is reasonable. Most important is to reference the GitHub issue that you are addressing with the PR.
NOTE: GitHub has a feature that will automatically close issues referenced with a keyword (such as "Fixes") by a PR or commit once the PR/commit is merged. Don't use these keywords. We don't want issues to be automatically closed. We want our testers to independently verify and close them.
Generally, pull requests should consist of a single logical commit. However, if your PR is for a large feature, you may need a more logical breakdown of commits. This is fine as long as each commit is a single logical unit.
Git commit messages should explain the how and why of your change and be separated into a brief subject line followed by a more detailed body. When in doubt, follow this guide for good commit messages and you can’t go wrong: https://chris.beams.io/posts/git-commit/.
One particularly useful point made in the above guide is regarding commit subject lines:
A properly formed Git commit subject line should always be able to complete the following sentence:
- If applied, this commit will your subject line here
A simple but effective convention to follow for commits is the “problem / solution” pattern. It looks like this:
<Subject>
Problem: <Statement of problem>
Solution: <Statement of solution>
As an example, here is a commit taken from the rancher/rancher repo:
commit b71ce2892eecb7c87a5212e3486f1de899a694aa
Author: Dan Ramich <[email protected]>
Date: Tue Jun 19 11:56:52 2018 -0700
Add Validator for RoleTemplate
Problem:
Builtin RoleTemplates can be updated through the API
Solution:
Add a Validator to ensure the only field that can be changed on a
builtin RoleTemplate is 'locked'
Generally, pull requests need one approval from maintainers to be merged.
When addressing review feedback, it is helpful to the reviewer if additional changes are made in new commits. This allows the reviewer to easily see the delta between what they previously reviewed and the changes you added to address their feedback. These commits should be in the form of 'fixups' ('git commit --fixup HEAD').
Once a PR has the necessary approvals, it can be merged.
Here’s how the merge should be handled:
- Once approved the author will be asked to squash all fixup commits generated by the review process.
- Commits and their messages should be consistent - each commit in the PR should form a logical unit with working code.
- The first change requested by a Jitsuin reviewer will be to reorganise the commits into a clean logical structure.
- The smaller a PR the more likely and more easily that the change will be approved.
- Any changes requested by a reviewer should be committed as a 'fixup' commit against the original commit in the PR.
- Once approval is granted any 'fixup' commits should be merged into their respective commits using 'git rebase -i --autosquash'.
To contribute to this project, you must agree to the Developer Certificate of Origin (DCO) for each commit you make. The DCO is a simple statement that you, as a contributor, have the legal right to make the contribution.
See the DCO file for the full text of what you must agree to.
To signify that you agree to the DCO for a commit, you add a line to the git commit message:
Signed-off-by: Jane Smith <[email protected]>
In most cases, you can add this signoff to your commit automatically with the
-s
flag to git commit
. Please use your real name and a reachable email address.