Skip to content

Commit

Permalink
Update PULL_REQUEST_TEMPLATE and CONTRIBUTING docs. (paritytech#4908
Browse files Browse the repository at this point in the history
)

Co-authored-by: Bastian Köcher <[email protected]>
  • Loading branch information
kianenigma and bkchr authored Jul 3, 2024
1 parent 18ed309 commit b69fc17
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 93 deletions.
132 changes: 59 additions & 73 deletions docs/contributor/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,16 @@ The `Polkadot SDK` project is an **OPENISH Open Source Project**

## What?

Individuals making significant and valuable contributions are given commit-access to the project.
Contributions are done via pull-requests and need to be approved by the maintainers.
Individuals making significant and valuable contributions are given commit-access to the project. Contributions are done
via pull-requests and need to be approved by the maintainers.

## Rules

There are a few basic ground-rules for contributors (including the maintainer(s) of the project):

1. **No `--force` pushes** or modifying the master branch history in any way.
If you need to rebase, ensure you do it in your own repo. No rewriting of the history
after the code has been shared (e.g. through a Pull-Request).
2. **Non-master branches**, prefixed with a short name moniker (e.g. `gav-my-feature`) must be
used for ongoing work.
1. **No `--force` pushes** or modifying the master branch history in any way. If you need to rebase, ensure you do it in
your own repo. No rewriting of the history after the code has been shared (e.g. through a Pull-Request).
2. **Non-master branches**, prefixed with a short name moniker (e.g. `gav-my-feature`) must be used for ongoing work.
3. **All modifications** must be made in a **pull-request** to solicit feedback from other contributors.
4. A pull-request **must not be merged until CI** has finished successfully.
5. Contributors should adhere to the [house coding style](./STYLE_GUIDE.md).
Expand All @@ -25,52 +23,37 @@ There are a few basic ground-rules for contributors (including the maintainer(s)

### In General

A Pull Request (PR) needs to be reviewed and approved by project maintainers.
If a change does not alter any logic (e.g. comments, dependencies, docs), then it may be tagged
`A1-insubstantial` and merged faster.
If it is an urgent fix with no large change to logic, then it may be merged after a non-author
contributor has reviewed it well and approved the review once CI is complete.
No PR should be merged until all reviews' comments are addressed.
* A Pull Request (PR) needs to be reviewed and approved by project maintainers.
* If a change does not alter any logic (e.g. comments, dependencies, docs), then it may be tagged `A1-insubstantial` and
merged faster.
* No PR should be merged until all reviews' comments are addressed.

### Labels

The set of labels and their description can be found [here](https://paritytech.github.io/labels/doc_polkadot-sdk.html).

### Process

1. Please use our [Pull Request Template](./PULL_REQUEST_TEMPLATE.md) and make sure all relevant
information is reflected in your PR.
2. Please tag each PR with minimum one `T*` label. The respective `T*` labels should signal the
component that was changed, they are also used by downstream users to track changes and to
include these changes properly into their own releases.
3. If you’re still working on your PR, please submit as “Draft”. Once a PR is ready for review change
the status to “Open”, so that the maintainers get to review your PR. Generally PRs should sit for
48 hours in order to garner feedback. It may be merged before if all relevant parties had a look at it.
4. If you’re introducing a major change, that might impact the documentation please add the label
`T13-documentation`. The docs team will get in touch.
5. If your PR changes files in these paths:

`polkadot` : `^runtime/polkadot`
`polkadot` : `^runtime/kusama`
`polkadot` : `^primitives/src/`
`polkadot` : `^runtime/common`
`substrate` : `^frame/`
`substrate` : `^primitives/`

It should be added to the [security audit board](https://github.com/orgs/paritytech/projects/103)
and will need to undergo an audit before merge.
6. PRs will be able to be merged once all reviewers' comments are addressed and CI is successful.

**Noting breaking changes:**
When breaking APIs, the PR description should mention what was changed alongside some examples on how
to change the code to make it work/compile.
It should also mention potential storage migrations and if they require some special setup aside adding
it to the list of migrations in the runtime.
1. Please use our [Pull Request Template](./PULL_REQUEST_TEMPLATE.md) and make sure all relevant information is
reflected in your PR.
2. Please tag each PR with minimum one `T*` label. The respective `T*` labels should signal the component that was
changed, they are also used by downstream users to track changes and to include these changes properly into their own
releases.
3. If you’re still working on your PR, please submit as “Draft”. Once a PR is ready for review change the status to
“Open”, so that the maintainers get to review your PR. Generally PRs should sit for 48 hours in order to garner
feedback. It may be merged before if all relevant parties had a look at it.
4. With respect to auditing, please see [AUDIT.md](../AUDIT.md). In general, merging to master can happen independent of
audit.
5. PRs will be able to be merged once all reviewers' comments are addressed and CI is successful.

**Noting breaking changes:** When breaking APIs, the PR description should mention what was changed alongside some
examples on how to change the code to make it work/compile. It should also mention potential storage migrations and if
they require some special setup aside adding it to the list of migrations in the runtime.

## Reviewing pull requests

When reviewing a pull request, the end-goal is to suggest useful changes to the author.
Reviews should finish with approval unless there are issues that would result in:
When reviewing a pull request, the end-goal is to suggest useful changes to the author. Reviews should finish with
approval unless there are issues that would result in:
1. Buggy behavior.
2. Undue maintenance burden.
3. Breaking with house coding style.
Expand All @@ -80,18 +63,17 @@ Reviews should finish with approval unless there are issues that would result in

The reviewers are also responsible to check:

1. if a changelog is necessary and attached
1. the quality of information in the changelog file
1. the PR has an impact on docs
1. that the docs team was included in the review process of a docs update
* if the PR description is well written to facilitate integration, in case it contains breaking changes.
* the PR has an impact on docs.

**Reviews may not be used as an effective veto for a PR because**:
1. There exists a somewhat cleaner/better/faster way of accomplishing the same feature/fix.
2. It does not fit well with some other contributors' longer-term vision for the project.

## Documentation
## `PRDoc`

All Pull Requests must contain proper title & description.
All Pull Requests must contain proper title & description, as described in [Pull Request
Template](./PULL_REQUEST_TEMPLATE.md). Moreover, all pull requests must have a proper `prdoc` file attached.

Some Pull Requests can be exempt of `prdoc` documentation, those must be labelled with
[`R0-silent`](https://github.com/paritytech/labels/blob/main/ruled_labels/specs_polkadot-sdk.yaml#L89-L91).
Expand All @@ -102,46 +84,49 @@ See more about `prdoc` [here](./prdoc.md)

## Helping out

We use [labels](https://github.com/paritytech/polkadot-sdk/labels) to manage PRs and issues and communicate
state of a PR. Please familiarise yourself with them. Best way to get started is to a pick a ticket tagged
[easy](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AD0-easy)
or [medium](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AD1-medium)
and get going. Alternatively, look out for issues tagged [mentor](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AC1-mentor)
and get in contact with the mentor offering their support on that larger task.
We use [labels](https://github.com/paritytech/polkadot-sdk/labels) to manage PRs and issues and communicate state of a
PR. Please familiarise yourself with them. Best way to get started is to a pick a ticket tagged
[easy](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AD0-easy) or
[medium](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AD1-medium) and get going.
Alternatively, look out for issues tagged
[mentor](https://github.com/paritytech/polkadot-sdk/issues?q=is%3Aopen+is%3Aissue+label%3AC1-mentor) and get in contact
with the mentor offering their support on that larger task.

****

### Issues

If what you are looking for is an answer rather than proposing a new feature or fix, search
[https://substrate.stackexchange.com](https://substrate.stackexchange.com/) to see if an post already
exists, and ask if not. Please do not file support issues here.
Before opening a new issue search to see if a similar one already exists and leave a comment that you
also experienced this issue or add your specifics that are related to an existing issue.
Please label issues with the following labels:
[https://substrate.stackexchange.com](https://substrate.stackexchange.com/) to see if an post already exists, and ask if
not. Please do not file support issues here.

Before opening a new issue search to see if a similar one already exists and leave a comment that you also experienced
this issue or add your specifics that are related to an existing issue.

Please label issues with the following labels (only relevant for maintainer):
1. `I*` issue severity and type. EXACTLY ONE REQUIRED.
2. `D*` issue difficulty, suggesting the level of complexity this issue has. AT MOST ONE ALLOWED.
3. `T*` Issue topic. MULTIPLE ALLOWED.

## Releases

Declaring formal releases remains the prerogative of the project maintainer(s).
Declaring formal releases remains the prerogative of the project maintainer(s). See [RELEASE.md](../RELEASE.md).

## UI tests

UI tests are used for macros to ensure that the output of a macro doesn’t change and is in the expected format.
These UI tests are sensible to any changes in the macro generated code or to switching the rust stable version.
The tests are only run when the `RUN_UI_TESTS` environment variable is set. So, when the CI is for example complaining
about failing UI tests and it is expected that they fail these tests need to be executed locally.
To simplify the updating of the UI test output there is a script
- `./scripts/update-ui-tests.sh` to update the tests for a current rust version locally
- `./scripts/update-ui-tests.sh 1.70` # to update the tests for a specific rust version locally
UI tests are used for macros to ensure that the output of a macro doesn’t change and is in the expected format. These UI
tests are sensible to any changes in the macro generated code or to switching the rust stable version. The tests are
only run when the `RUN_UI_TESTS` environment variable is set. So, when the CI is for example complaining about failing
UI tests and it is expected that they fail these tests need to be executed locally. To simplify the updating of the UI
test output there is a script
* `./scripts/update-ui-tests.sh` to update the tests for a current rust version locally
* `./scripts/update-ui-tests.sh 1.70` # to update the tests for a specific rust version locally

Or if you have opened PR and you're member of `paritytech` - you can use command-bot to run the tests for you in CI:
- `bot update-ui` - will run the tests for the current rust version
- `bot update-ui latest --rust_version=1.70.0` - will run the tests for the specified rust version
- `bot update-ui latest -v CMD_IMAGE=paritytech/ci-unified:bullseye-1.70.0-2023-05-23 --rust_version=1.70.0` -
will run the tests for the specified rust version and specified image
* `bot update-ui` - will run the tests for the current rust version
* `bot update-ui latest --rust_version=1.70.0` - will run the tests for the specified rust version
* `bot update-ui latest -v CMD_IMAGE=paritytech/ci-unified:bullseye-1.70.0-2023-05-23 --rust_version=1.70.0` - will run
the tests for the specified rust version and specified image

## Feature Propagation

Expand All @@ -157,4 +142,5 @@ Start with comment in PR: `bot help` to see the list of available commands.
## Deprecating code

When deprecating and removing code you need to be mindful of how this could impact downstream developers. In order to
mitigate this impact, it is recommended to adhere to the steps outlined in the [Deprecation Checklist](./DEPRECATION_CHECKLIST.md).
mitigate this impact, it is recommended to adhere to the steps outlined in the [Deprecation
Checklist](./DEPRECATION_CHECKLIST.md).
43 changes: 25 additions & 18 deletions docs/contributor/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,42 @@

✄ -----------------------------------------------------------------------------

Thank you for your Pull Request! 🙏 Please make sure it follows the contribution guidelines outlined in
[this document](https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md) and fill
out the sections below. Once you're ready to submit your PR for review, please
delete this section and leave only the text under the "Description" heading.
Thank you for your Pull Request! 🙏 Please make sure it follows the contribution guidelines outlined in [this
document](https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md) and fill out the
sections below. Once you're ready to submit your PR for review, please delete this section and leave only the text under
the "Description" heading.

# Description

*Please include a summary of the changes and the related issue. Please also include relevant motivation and context,
including:*
*A concise description of what your PR is doing, and what potential issue it is solving. Use [Github semantic
linking](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)
to link the PR to an issue that must be closed once this is merged.*

- What does this PR do?
- Why are these changes needed?
- How were these changes implemented and what do they affect?
## Integration

*Use [Github semantic
linking](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)
to address any open issues this PR relates to or closes.*
*In depth notes about how this PR should be integrated by downstream projects. This part is mandatory, and should be
reviewed by reviewers, if the PR does NOT have the `R0-Silent` label. In case of a `R0-Silent`, it can be ignored.*

## Review Notes

*In depth notes about the **implenentation** details of your PR. This should be the main guide for reviewers to
understand your approach and effectively review it. If too long, use
[`<details>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details)*.

Fixes # (issue number, *if applicable*)
*Imagine that someone who is depending on the old code wants to integrate your new code and the only information that
they get is this section. It helps to include example usage and default value here, with a `diff` code-block to show
possibly integration.*

Closes # (issue number, *if applicable*)
*Include your leftover TODOs, if any, here.*

# Checklist

- [ ] My PR includes a detailed description as outlined in the "Description" section above
- [ ] My PR follows the [labeling requirements](CONTRIBUTING.md#Process) of this project (at minimum one label for `T`
* [ ] My PR includes a detailed description as outlined in the "Description" and its two subsections above.
* [ ] My PR follows the [labeling requirements](CONTRIBUTING.md#Process) of this project (at minimum one label for `T`
required)
- [ ] I have made corresponding changes to the documentation (if applicable)
- [ ] I have added tests that prove my fix is effective or that my feature works (if applicable)
* External contributors: ask maintainers to put the right label on your PR.
* [ ] I have made corresponding changes to the documentation (if applicable)
* [ ] I have added tests that prove my fix is effective or that my feature works (if applicable)

You can remove the "Checklist" section once all have been checked. Thank you for your contribution!

Expand Down
4 changes: 2 additions & 2 deletions templates/minimal/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ packages required to compile this template - please take note of the Rust compil

## Customization

In case you would like to change the pallet's name from `pallet-minimal-template` to `pallet-test`,
In case you would like to change the pallet's name from `pallet-minimal-template` to `pallet-test`,
you would need to implement the following changes:
* Change the pallet folder name from `template` to `test` for consistency
* Also for consistency, in `/runtime/src/lib.rs`, change from `pub type Template` to `pub type Test`,
* Also for consistency, in `/runtime/src/lib.rs`, change from `pub type Template` to `pub type Test`,
and don't forget to also customize the corresponding comments.
* Change `pallet-minimal-template` to `pallet-test` in the following files:
* `/runtime/Cargo.toml`
Expand Down

0 comments on commit b69fc17

Please sign in to comment.