|
| 1 | +Contributing guidelines |
| 2 | +======================= |
| 3 | + |
| 4 | +Contributions are very welcome. When contributing code, please follow these |
| 5 | +simple guidelines. |
| 6 | + |
| 7 | +#### Table Of Contents |
| 8 | +- [Contribution workflow](#contribution-workflow) |
| 9 | + * [Proposing changes](#proposing-changes) |
| 10 | + * [Preparing PRs](#preparing-prs) |
| 11 | + * [Peer review](#peer-review) |
| 12 | +- [Coding conventions](#coding-conventions) |
| 13 | +- [Security](#security) |
| 14 | +- [Testing](#testing) |
| 15 | +- [Going further](#going-further) |
| 16 | + |
| 17 | +Overview |
| 18 | +-------- |
| 19 | + |
| 20 | +* Before adding any code dependencies, check with the maintainers if this is okay. |
| 21 | +* Write properly formatted comments: they should be English sentences, eg: |
| 22 | + |
| 23 | + // Return the current UNIX time. |
| 24 | + |
| 25 | +* Read the DCO and make sure all commits are signed off, using `git commit -s`. |
| 26 | +* Follow the guidelines when proposing code changes (see below). |
| 27 | +* Write properly formatted git commits (see below). |
| 28 | +* Run the tests with `cargo test --workspace --all-features`. |
| 29 | +* Make sure you run `rustfmt` on your code (see below details). |
| 30 | +* Please don't file an issue to ask a question. Each repository - or |
| 31 | + GitHub organization has a "Discussions" with Q&A section; please post your |
| 32 | + questions there. You'll get faster results by using this channel. |
| 33 | + |
| 34 | +Contribution Workflow |
| 35 | +--------------------- |
| 36 | +The codebase is maintained using the "contributor workflow" where everyone |
| 37 | +without exception contributes patch proposals using "pull requests". This |
| 38 | +facilitates social contribution, easy testing and peer review. |
| 39 | + |
| 40 | +To contribute a patch, the workflow is a as follows: |
| 41 | + |
| 42 | +1. Fork Repository |
| 43 | +2. Create topic branch |
| 44 | +3. Commit patches |
| 45 | + |
| 46 | +In general commits should be atomic and diffs should be easy to read. For this |
| 47 | +reason do not mix any formatting fixes or code moves with actual code changes. |
| 48 | +Further, each commit, individually, should compile and pass tests, in order to |
| 49 | +ensure git bisect and other automated tools function properly. |
| 50 | + |
| 51 | +When adding a new feature thought must be given to the long term technical debt. |
| 52 | +Every new features should be covered by unit tests. |
| 53 | + |
| 54 | +When refactoring, structure your PR to make it easy to review and don't hesitate |
| 55 | +to split it into multiple small, focused PRs. |
| 56 | + |
| 57 | +Commits should cover both the issue fixed and the solution's rationale. |
| 58 | +These [guidelines](https://chris.beams.io/posts/git-commit/) should be kept in |
| 59 | +mind. |
| 60 | + |
| 61 | +To facilitate communication with other contributors, the project is making use |
| 62 | +of GitHub's "assignee" field. First check that no one is assigned and then |
| 63 | +comment suggesting that you're working on it. If someone is already assigned, |
| 64 | +don't hesitate to ask if the assigned party or previous commenters are still |
| 65 | +working on it if it has been awhile. |
| 66 | + |
| 67 | +### Proposing changes |
| 68 | + |
| 69 | +When proposing changes via a pull-request or patch: |
| 70 | + |
| 71 | +* Isolate changes in separate commits to make the review process easier. |
| 72 | +* Don't make unrelated changes, unless it happens to be an obvious improvement to |
| 73 | + code you are touching anyway ("boyscout rule"). |
| 74 | +* Rebase on `master` when needed. |
| 75 | +* Keep your changesets small, specific and uncontroversial, so that they can be |
| 76 | + merged more quickly. |
| 77 | +* If the change is substantial or requires re-architecting certain parts of the |
| 78 | + codebase, write a proposal in English first, and get consensus on that before |
| 79 | + proposing the code changes. |
| 80 | + |
| 81 | +### Preparing PRs |
| 82 | + |
| 83 | +The main library development happens in the `master` branch. This branch must |
| 84 | +always compile without errors (using Travis CI). All external contributions are |
| 85 | +made within PRs into this branch. |
| 86 | + |
| 87 | +Prerequisites that a PR must satisfy for merging into the `master` branch: |
| 88 | +* the tip of any PR branch must compile and pass unit tests with no errors, with |
| 89 | + every feature combination (including compiling the fuzztests) on MSRV, stable |
| 90 | + and nightly compilers (this is partially automated with CI, so the rule |
| 91 | + is that we will not accept commits which do not pass GitHub CI); |
| 92 | +* contain all necessary tests for the introduced functional (either as a part of |
| 93 | + commits, or, more preferably, as separate commits, so that it's easy to |
| 94 | + reorder them during review and check that the new tests fail without the new |
| 95 | + code); |
| 96 | +* contain all inline docs for newly introduced API and pass doc tests; |
| 97 | +* be based on the recent `master` tip from the original repository at. |
| 98 | + |
| 99 | +NB: reviewers may run more complex test/CI scripts, thus, satisfying all the |
| 100 | +requirements above is just a preliminary, but not necessary sufficient step for |
| 101 | +getting the PR accepted as a valid candidate PR for the `master` branch. |
| 102 | + |
| 103 | +Additionally, to the `master` branch some repositories may have `develop` branch |
| 104 | +for any experimental developments. This branch may not compile and should not be |
| 105 | +used by any projects depending on the library. |
| 106 | + |
| 107 | +### Writing Git commit messages |
| 108 | + |
| 109 | +A properly formed git commit subject line should always be able to complete the |
| 110 | +following sentence: |
| 111 | + |
| 112 | + If applied, this commit will _____ |
| 113 | + |
| 114 | +In addition, it should be capitalized and *must not* include a period. |
| 115 | + |
| 116 | +For example, the following message is well formed: |
| 117 | + |
| 118 | + Add support for .gif files |
| 119 | + |
| 120 | +While these ones are **not**: `Adding support for .gif files`, |
| 121 | +`Added support for .gif files`. |
| 122 | + |
| 123 | +When it comes to formatting, here's a model git commit message[1]: |
| 124 | + |
| 125 | + Capitalized, short (50 chars or less) summary |
| 126 | + |
| 127 | + More detailed explanatory text, if necessary. Wrap it to about 72 |
| 128 | + characters or so. In some contexts, the first line is treated as the |
| 129 | + subject of an email and the rest of the text as the body. The blank |
| 130 | + line separating the summary from the body is critical (unless you omit |
| 131 | + the body entirely); tools like rebase can get confused if you run the |
| 132 | + two together. |
| 133 | + |
| 134 | + Write your commit message in the imperative: "Fix bug" and not "Fixed bug" |
| 135 | + or "Fixes bug." This convention matches up with commit messages generated |
| 136 | + by commands like git merge and git revert. |
| 137 | + |
| 138 | + Further paragraphs come after blank lines. |
| 139 | + |
| 140 | + - Bullet points are okay, too. |
| 141 | + |
| 142 | + - Typically a hyphen or asterisk is used for the bullet, followed by a |
| 143 | + single space, with blank lines in between, but conventions vary here. |
| 144 | + |
| 145 | + - Use a hanging indent. |
| 146 | + |
| 147 | +### Peer review |
| 148 | + |
| 149 | +Anyone may participate in peer review which is expressed by comments in the pull |
| 150 | +request. Typically reviewers will review the code for obvious errors, as well as |
| 151 | +test out the patch set and opine on the technical merits of the patch. PR should |
| 152 | +be reviewed first on the conceptual level before focusing on code style or |
| 153 | +grammar fixes. |
| 154 | + |
| 155 | +Coding Conventions |
| 156 | +------------------ |
| 157 | +Our CI enforces [clippy's](https://github.com/rust-lang/rust-clippy) |
| 158 | +[default linting](https://rust-lang.github.io/rust-clippy/rust-1.52.0/index.html) |
| 159 | +and [rustfmt](https://github.com/rust-lang/rustfmt) formatting defined by rules |
| 160 | +in [.rustfmt.toml](./.rustfmt.toml). The linter should be run with current |
| 161 | +stable rust compiler, while formatter requires nightly version due to the use of |
| 162 | +unstable formatting parameters. |
| 163 | + |
| 164 | +If you use rustup, to lint locally you may run the following instructions: |
| 165 | + |
| 166 | +```console |
| 167 | +rustup component add clippy |
| 168 | +rustup component add fmt |
| 169 | +cargo +stable clippy --workspace --all-features |
| 170 | +cargo +nightly fmt --all |
| 171 | +``` |
| 172 | + |
| 173 | +Security |
| 174 | +-------- |
| 175 | +Responsible disclosure of security vulnerabilities helps prevent user loss of |
| 176 | +privacy. If you believe a vulnerability may affect other implementations, please |
| 177 | +inform them. Guidelines for a responsible disclosure can be found in |
| 178 | +[SECURITY.md](./SECURITY.md) file in the project root. |
| 179 | + |
| 180 | +Note that some of our projects are currently considered "pre-production". |
| 181 | +Such projects can be distinguished by the absence of `SECURITY.md`. In such |
| 182 | +cases there are no special handling of security issues; please simply open |
| 183 | +an issue on GitHub. |
| 184 | + |
| 185 | +Going further |
| 186 | +------------- |
| 187 | +You may be interested in Jon Atack guide on |
| 188 | +[How to review Bitcoin Core PRs][Review] and [How to make Bitcoin Core PRs][PR]. |
| 189 | +While there are differences between the projects in terms of context and |
| 190 | +maturity, many of the suggestions offered apply to this project. |
| 191 | + |
| 192 | +Overall, have fun :) |
| 193 | + |
| 194 | +[1]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html |
| 195 | +[Review]: https://github.com/jonatack/bitcoin-development/blob/master/how-to-review-bitcoin-core-prs.md |
| 196 | +[PR]: https://github.com/jonatack/bitcoin-development/blob/master/how-to-make-bitcoin-core-prs.md |
0 commit comments