|
| 1 | +# Contributing to jitsuin-archivist # |
| 2 | + |
| 3 | +Thanks for taking the time to contribute to jitsuin-archivist! |
| 4 | + |
| 5 | +Contributing is not limited to writing code and submitting a PR. Feel free to submit an |
| 6 | +[issue](https://github.com/jitsuin-inc/archivist-python/issues/new/choose) or comment on an existing one |
| 7 | +to report a bug, provide feedback, or suggest a new feature. |
| 8 | + |
| 9 | +Of course, contributing code is more than welcome! To keep things simple, if you're fixing a small issue, |
| 10 | +you can simply submit a PR and we will pick it up. However, if you're planning to submit a bigger PR to implement |
| 11 | +a new feature or fix a relatively complex bug, please open an issue that explains the change and the motivation for it. |
| 12 | +If you're addressing a bug, please explain how to reproduce it. |
| 13 | + |
| 14 | +## Pull request and git commit guidance |
| 15 | + |
| 16 | +### Opening PRs and organizing commits |
| 17 | +PRs should generally address only 1 issue at a time. If you need to fix two bugs, open two separate PRs. |
| 18 | +This will keep the scope of your pull requests smaller and allow them to be reviewed and merged more quickly. |
| 19 | + |
| 20 | +When possible, fill out as much detail in the pull request template as is reasonable. |
| 21 | +Most important is to reference the GitHub issue that you are addressing with the PR. |
| 22 | + |
| 23 | +**NOTE:** GitHub has |
| 24 | +[a feature](https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword) |
| 25 | +that will automatically close issues referenced with a keyword (such as "Fixes") by a PR or commit once the PR/commit is merged. |
| 26 | +Don't use these keywords. We don't want issues to be automatically closed. We want our testers to independently verify and close them. |
| 27 | + |
| 28 | +Generally, pull requests should consist of a single logical commit. |
| 29 | +However, if your PR is for a large feature, you may need a more logical breakdown of commits. |
| 30 | +This is fine as long as each commit is a single logical unit. |
| 31 | + |
| 32 | +### Writing good commit messages |
| 33 | +Git commit messages should explain the how and why of your change and be separated into a brief subject line |
| 34 | +followed by a more detailed body. When in doubt, follow this guide for good commit messages and |
| 35 | +you can’t go wrong: https://chris.beams.io/posts/git-commit/. |
| 36 | + |
| 37 | +One particularly useful point made in the above guide is regarding commit subject lines: |
| 38 | + |
| 39 | +> A properly formed Git commit subject line should always be able to complete the following sentence: |
| 40 | +> |
| 41 | +> - If applied, this commit will <ins>your subject line here</ins> |
| 42 | +
|
| 43 | +A simple but effective convention to follow for commits is the “problem / solution” pattern. It looks like this: |
| 44 | +``` |
| 45 | +<Subject> |
| 46 | +
|
| 47 | +Problem: <Statement of problem> |
| 48 | +
|
| 49 | +Solution: <Statement of solution> |
| 50 | +``` |
| 51 | + |
| 52 | +As an example, here is a commit taken from the rancher/rancher repo: |
| 53 | +``` |
| 54 | +commit b71ce2892eecb7c87a5212e3486f1de899a694aa |
| 55 | +Author: Dan Ramich <danold215@gmail.com> |
| 56 | +Date: Tue Jun 19 11:56:52 2018 -0700 |
| 57 | +
|
| 58 | + Add Validator for RoleTemplate |
| 59 | +
|
| 60 | + Problem: |
| 61 | + Builtin RoleTemplates can be updated through the API |
| 62 | +
|
| 63 | + Solution: |
| 64 | + Add a Validator to ensure the only field that can be changed on a |
| 65 | + builtin RoleTemplate is 'locked' |
| 66 | +``` |
| 67 | + |
| 68 | +### Reviewing, addressing feedback, and merging |
| 69 | +Generally, pull requests need two approvals from maintainers to be merged. |
| 70 | + |
| 71 | +When addressing review feedback, it is helpful to the reviewer if additional changes are made in new commits. |
| 72 | +This allows the reviewer to easily see the delta between what they previously reviewed and the changes you added |
| 73 | +to address their feedback. |
| 74 | + |
| 75 | +Once a PR has the necessary approvals, it can be merged. |
| 76 | +Here’s how the merge should be handled: |
| 77 | +- All PR's should use the “Squash and merge” option. |
| 78 | +- Commits and their messages should be consistent - each commit in the PR should form a logical unit with working code. |
| 79 | +- The first change requested by a Jitsuin reviewer will be to reorganise the commits into a clean logical structure. |
| 80 | +- The smaller a PR the more likely and more easily that the change will be approved. |
| 81 | +- Any changes requested by a reviewer should be committed as a 'fixup' commit against the original commit in the PR. |
| 82 | +- Once approval is granted any 'fixup' commits should be merged into their respective commits using 'git rebase -i --autosquash'. |
| 83 | + |
| 84 | +## Developer Certificate Of Origin ## |
| 85 | + |
| 86 | +To contribute to this project, you must agree to the Developer Certificate of Origin (DCO) for each commit you make. |
| 87 | +The DCO is a simple statement that you, as a contributor, have the legal right to make the contribution. |
| 88 | + |
| 89 | +See the [DCO](DCO) file for the full text of what you must agree to. |
| 90 | + |
| 91 | +To signify that you agree to the DCO for a commit, you add a line to the git |
| 92 | +commit message: |
| 93 | + |
| 94 | +```txt |
| 95 | +Signed-off-by: Jane Smith <jane.smith@example.com> |
| 96 | +``` |
| 97 | + |
| 98 | +In most cases, you can add this signoff to your commit automatically with the |
| 99 | +`-s` flag to `git commit`. Please use your real name and a reachable email address. |
0 commit comments