|
| 1 | +# Contributing |
| 2 | + |
| 3 | +The `Polkadot SDK` project is an **OPENISH Open Source Project** |
| 4 | + |
| 5 | +## What? |
| 6 | + |
| 7 | +Individuals making significant and valuable contributions are given commit-access to the project. |
| 8 | +Contributions are done via pull-requests and need to be approved by the maintainers. |
| 9 | + |
| 10 | +## Rules |
| 11 | + |
| 12 | +There are a few basic ground-rules for contributors (including the maintainer(s) of the project): |
| 13 | + |
| 14 | +1. **No `--force` pushes** or modifying the master branch history in any way. |
| 15 | + If you need to rebase, ensure you do it in your own repo. No rewriting of the history |
| 16 | + after the code has been shared (e.g. through a Pull-Request). |
| 17 | +2. **Non-master branches**, prefixed with a short name moniker (e.g. `gav-my-feature`) must be |
| 18 | + used for ongoing work. |
| 19 | +3. **All modifications** must be made in a **pull-request** to solicit feedback from other contributors. |
| 20 | +4. A pull-request **must not be merged until CI** has finished successfully. |
| 21 | +5. Contributors should adhere to the [house coding style](./STYLE_GUIDE.md). |
| 22 | +6. Contributors should adhere to the [house documenting style](./DOCUMENTATION_GUIDELINES.md), when applicable. |
| 23 | + |
| 24 | +## Merge Process |
| 25 | + |
| 26 | +### In General |
| 27 | + |
| 28 | +A Pull Request (PR) needs to be reviewed and approved by project maintainers. |
| 29 | +If a change does not alter any logic (e.g. comments, dependencies, docs), then it may be tagged |
| 30 | +`A1-insubstantial` and merged faster. |
| 31 | +If it is an urgent fix with no large change to logic, then it may be merged after a non-author |
| 32 | +contributor has reviewed it well and approved the review once CI is complete. |
| 33 | +No PR should be merged until all reviews' comments are addressed. |
| 34 | + |
| 35 | +### Labels: |
| 36 | + |
| 37 | +The set of labels and their description can be found [here](linktobeinserted) |
| 38 | + |
| 39 | +### Process: |
| 40 | + |
| 41 | +1. Please use our [Pull Request Template](./PULL_REQUEST_TEMPLATE.md) and make sure all relevant |
| 42 | + information is reflected in your PR. |
| 43 | +2. Please tag each PR with minimum one `T*` label. The respective `T*` labels should signal the |
| 44 | + component that was changed, they are also used by downstream users to track changes and to |
| 45 | + include these changes properly into their own releases. |
| 46 | +3. If your’re still working on your PR, please submit as “Draft”. Once a PR is ready for review change |
| 47 | + the status to “Open”, so that the maintainers get to review your PR. Generally PRs should sit for |
| 48 | + 48 hours in order to garner feedback. It may be merged before if all relevant parties had a look at it. |
| 49 | +4. If you’re introducing a major change, that might impact the documentation please add the label |
| 50 | + `T13-documentation`. The docs team will get in touch. |
| 51 | +5. If your PR changes files in these paths: |
| 52 | + |
| 53 | + `polkadot` : '^runtime/polkadot' |
| 54 | + `polkadot` : '^runtime/kusama' |
| 55 | + `polkadot` : '^primitives/src/' |
| 56 | + `polkadot` : '^runtime/common' |
| 57 | + `substrate` : '^frame/' |
| 58 | + `substrate` : '^primitives/' |
| 59 | + |
| 60 | + It should be added to the [security audit board](https://github.com/orgs/paritytech/projects/103) |
| 61 | + and will need to undergo an audit before merge. |
| 62 | +6. PRs will be able to be merged once all reviewers' comments are addressed and CI is successful. |
| 63 | + |
| 64 | +**Noting breaking changes:** |
| 65 | +When breaking APIs, the PR description should mention what was changed alongside some examples on how |
| 66 | +to change the code to make it work/compile. |
| 67 | +It should also mention potential storage migrations and if they require some special setup aside adding |
| 68 | +it to the list of migrations in the runtime. |
| 69 | + |
| 70 | +## Reviewing pull requests: |
| 71 | + |
| 72 | +When reviewing a pull request, the end-goal is to suggest useful changes to the author. |
| 73 | +Reviews should finish with approval unless there are issues that would result in: |
| 74 | +1. Buggy behavior. |
| 75 | +2. Undue maintenance burden. |
| 76 | +3. Breaking with house coding style. |
| 77 | +4. Pessimization (i.e. reduction of speed as measured in the projects benchmarks). |
| 78 | +5. Feature reduction (i.e. it removes some aspect of functionality that a significant minority of users rely on). |
| 79 | +6. Uselessness (i.e. it does not strictly add a feature or fix a known issue). |
| 80 | + |
| 81 | +The reviewers are also responsible to check: |
| 82 | + |
| 83 | +a) if a changelog is necessary and attached |
| 84 | + |
| 85 | +b) the quality of information in the changelog file |
| 86 | + |
| 87 | +c) the PR has an impact on docs |
| 88 | + |
| 89 | +d) that the docs team was included in the review process of a docs update |
| 90 | + |
| 91 | +**Reviews may not be used as an effective veto for a PR because**: |
| 92 | +1. There exists a somewhat cleaner/better/faster way of accomplishing the same feature/fix. |
| 93 | +2. It does not fit well with some other contributors' longer-term vision for the project. |
| 94 | + |
| 95 | +## Helping out |
| 96 | + |
| 97 | +We use [labels](https://github.com/paritytech/polkadot-sdk/labels) to manage PRs and issues and communicate |
| 98 | +state of a PR. Please familiarise yourself with them. Best way to get started is to a pick a ticket tagged |
| 99 | +`[easy](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AD0-easy)` |
| 100 | +or `[medium](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AD1-medium)` |
| 101 | +and get going or `[mentor](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AC1-mentor)` |
| 102 | +and get in contact with the mentor offering their support on that larger task. |
| 103 | + |
| 104 | +**** |
| 105 | + |
| 106 | +### Issues |
| 107 | + |
| 108 | +If what you are looking for is an answer rather than proposing a new feature or fix, search |
| 109 | +[https://substrate.stackexchange.com](https://substrate.stackexchange.com/) to see if an post already |
| 110 | +exists, and ask if not. Please do not file support issues here. |
| 111 | +Before opening a new issue search to see if a similar one already exists and leave a comment that you |
| 112 | +also experienced this issue or add your specifics that are related to an existing issue. |
| 113 | +Please label issues with the following labels: |
| 114 | +1. `I*` issue severity and type. EXACTLY ONE REQUIRED. |
| 115 | +2. `D*` issue difficulty, suggesting the level of complexity this issue has. AT MOST ONE ALLOWED. |
| 116 | +3. `T*` Issue topic. MULTIPLE ALLOWED. |
| 117 | + |
| 118 | +## Releases |
| 119 | + |
| 120 | +Declaring formal releases remains the prerogative of the project maintainer(s). |
| 121 | + |
| 122 | +## UI tests |
| 123 | + |
| 124 | +UI tests are used for macros to ensure that the output of a macro doesn’t change and is in the expected format. |
| 125 | +These UI tests are sensible to any changes in the macro generated code or to switching the rust stable version. |
| 126 | +The tests are only run when the `RUN_UI_TESTS` environment variable is set. So, when the CI is for example complaining |
| 127 | +about failing UI tests and it is expected that they fail these tests need to be executed locally. |
| 128 | +To simplify the updating of the UI test ouput there is the `.maintain/update-rust-stable |
0 commit comments