Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update contribution guidelines, remove redundant files #1181

Merged
merged 12 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cumulus/docs/documentation.md

This file was deleted.

File renamed without changes.
93 changes: 93 additions & 0 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# Contributing
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.
## 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.
the-right-joyce marked this conversation as resolved.
Show resolved Hide resolved
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).
6. Contributors should adhere to the [house documenting style](./DOCUMENTATION_GUIDELINES.md), when applicable.
## Merge Process
### 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.

### Labels:
The set of labels and their description can be found [here](linktobeinserted)

### 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 your’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.

## 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:
1. Buggy behavior.
2. Undue maintenance burden.
3. Breaking with house coding style.
4. Pessimization (i.e. reduction of speed as measured in the projects benchmarks).
5. Feature reduction (i.e. it removes some aspect of functionality that a significant minority of users rely on).
6. Uselessness (i.e. it does not strictly add a feature or fix a known issue).

The reviewers are also responsible to check:

a) if a changelog is necessary and attached

b) the quality of information in the changelog file

c) the PR has an impact on docs

d) that the docs team was included in the review process of a docs update
**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.

## 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 or `[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:
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).

## 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 ouput there is the `.maintain/update-rust-stable.sh` script. This can be run with `.maintain/update-rust-stable.sh CURRENT_STABLE_VERSION` and then it will run all UI tests to update the expected output. Otherwise you can also run a test with the command bot `bot update-ui CURRENT_STABLE_VERSION`.

## Changes to this arrangement
This is an experiment and feedback is welcome! This document may also be subject to pull-requests or changes by contributors where you believe you have something valuable to add or change.

## Heritage
These contributing guidelines are modified from the "OPEN Open Source Project" guidelines for the Level project: https://github.com/Level/community/blob/master/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ Use `# Examples as much as possible. These are great ways to further demonstrate

You can also consider having an `# Error` section optionally. Of course, this only applies if there is a `Result` being returned, and if the `Error` variants are overly complicated.

Strive to include correct links to other items in your written docs as much as possible. In other words, avoid \`some_func\` and instead use \[\`some_fund\`\]. Read more about how to correctly use links in your rust-docs [here](https://doc.rust-lang.org/rustdoc/write-documentation/linking-to-items-by-name.html#valid-links) and [here](https://rust-lang.github.io/rfcs/1946-intra-rustdoc-links.html#additions-to-the-documentation-syntax).
Strive to include correct links to other items in your written docs as much as possible. In other words, avoid `` `some_func` `` and instead use ``[`some_func`]``.
Read more about how to correctly use links in your rust-docs [here](https://doc.rust-lang.org/rustdoc/write-documentation/linking-to-items-by-name.html#valid-links) and [here](https://rust-lang.github.io/rfcs/1946-intra-rustdoc-links.html#additions-to-the-documentation-syntax).


> While you are linking, you might become conscious of the fact that you are in need of linking to (too many) foreign items in order to explain your API. This is leaning more towards API-Design rather than documentation, but it is a warning that the subject API might be slightly wrong. For example, most "glue" traits[^1] in `frame/support` should be designed and documented without making hard assumptions about particular pallets that implement them.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

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

Thank you for your Pull Request! 🙏 Please make sure it follows the contribution guidelines outlined in [this document](https://github.com/paritytech/substrate/blob/master/docs/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/substrate/blob/master/docs/CONTRIBUTING.adoc) 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.
the-right-joyce marked this conversation as resolved.
Show resolved Hide resolved

# Description

Expand All @@ -25,7 +25,7 @@ Cumulus companion: (*if applicable*)
# Checklist

- [ ] My PR includes a detailed description as outlined in the "Description" section above
- [ ] My PR follows the [labeling requirements](https://github.com/paritytech/substrate/blob/master/docs/CONTRIBUTING.md#merge-process) of this project (at minimum one label for each `A`, `B`, `C` and `D` required)
- [ ] My PR follows the [labeling requirements](https://github.com/paritytech/substrate/blob/master/docs/CONTRIBUTING.adoc#merge-process) of this project (at minimum one label for each `A`, `B`, `C` and `D` required)
the-right-joyce marked this conversation as resolved.
Show resolved Hide resolved
- [ ] 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)
- [ ] If this PR alters any external APIs or interfaces used by Polkadot, the corresponding Polkadot PR is ready as well as the corresponding Cumulus PR (optional)
Expand Down
7 changes: 4 additions & 3 deletions polkadot/SECURITY.md → docs/SECURITY.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<!-- markdown-link-check-disable -->
# Security Policy

Parity Technologies is committed to resolving security vulnerabilities in our software quickly and carefully. We take the necessary steps to minimize risk, provide timely information, and deliver vulnerability fixes and mitigations required to address security issues.

## Reporting a Vulnerability

Security vulnerabilities in Parity software should be reported by email to security@parity.io. If you think your report might be eligible for the Parity Bug Bounty Program, your email should be sent to bugbounty@parity.io.
Security vulnerabilities in Parity software should be reported by email to security@parity.io. If you think your report might be eligible for the Parity Bug Bounty Program, your email should be send to bugbounty@parity.io.

Your report should include the following:

Expand All @@ -19,7 +20,7 @@ Try to include as much information in your report as you can, including a descri

You'll receive a response to your email within two business days indicating the next steps in handling your report. We encourage finders to use encrypted communication channels to protect the confidentiality of vulnerability reports. You can encrypt your report using our public key. This key is [on MIT's key server](https://pgp.mit.edu/pks/lookup?op=get&search=0x5D0F03018D07DE73) server and reproduced below.

After the initial reply to your report, our team will endeavor to keep you informed of the progress being made towards a fix. These updates will be sent at least every five business days.
After the initial reply to your report, our team will endeavor to keep you informed of the progress being made towards a fix. These updates will be sent at least every five business days.

Thank you for taking the time to responsibly disclose any vulnerabilities you find.

Expand All @@ -36,7 +37,7 @@ Responsible investigation and reporting includes, but isn't limited to, the foll

## Bug Bounty Program

Our Bug Bounty Program allows us to recognise and reward members of the Parity community for helping us find and address significant bugs, in accordance with the terms of the Parity Bug Bounty Program. A detailed description on eligibility, rewards, legal information and terms & conditions for contributors can be found on [our website](https://paritytech.io/bug-bounty.html).
Our Bug Bounty Program allows us to recognize and reward members of the Parity community for helping us find and address significant bugs, in accordance with the terms of the Parity Bug Bounty Program. A detailed description on eligibility, rewards, legal information and terms & conditions for contributors can be found on [our website](https://paritytech.io/bug-bounty.html).



Expand Down
172 changes: 172 additions & 0 deletions docs/STYLE_GUIDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
---
title: Style Guide for Rust in Substrate
the-right-joyce marked this conversation as resolved.
Show resolved Hide resolved
---

Where possible these styles are enforced by settings in `rustfmt.toml` so if you run `cargo fmt`
then you will adhere to most of these style guidelines automatically.

# Formatting

- Indent using tabs.
- Lines should be longer than 100 characters long only in exceptional circumstances and certainly
no longer than 120. For this purpose, tabs are considered 4 characters wide.
- Indent levels should be greater than 5 only in exceptional circumstances and certainly no
greater than 8. If they are greater than 5, then consider using `let` or auxiliary functions in
order to strip out complex inline expressions.
- Never have spaces on a line prior to a non-whitespace character
- Follow-on lines are only ever a single indent from the original line.

```rust
fn calculation(some_long_variable_a: i8, some_long_variable_b: i8) -> bool {
let x = some_long_variable_a * some_long_variable_b
- some_long_variable_b / some_long_variable_a
+ sqrt(some_long_variable_a) - sqrt(some_long_variable_b);
x > 10
}
```

- Indent level should follow open parens/brackets, but should be collapsed to the smallest number
of levels actually used:

```rust
fn calculate(
some_long_variable_a: f32,
some_long_variable_b: f32,
some_long_variable_c: f32,
) -> f32 {
(-some_long_variable_b + sqrt(
// two parens open, but since we open & close them both on the
// same line, only one indent level is used
some_long_variable_b * some_long_variable_b
- 4 * some_long_variable_a * some_long_variable_c
// both closed here at beginning of line, so back to the original indent
// level
)) / (2 * some_long_variable_a)
}
```

- `where` is indented, and its items are indented one further.
- Argument lists or function invocations that are too long to fit on one line are indented
similarly to code blocks, and once one param is indented in such a way, all others should be,
too. Run-on parameter lists are also acceptable for single-line run-ons of basic function calls.

```rust
// OK
fn foo(
really_long_parameter_name_1: SomeLongTypeName,
really_long_parameter_name_2: SomeLongTypeName,
shrt_nm_1: u8,
shrt_nm_2: u8,
) {
...
}

// NOT OK
fn foo(really_long_parameter_name_1: SomeLongTypeName, really_long_parameter_name_2: SomeLongTypeName,
shrt_nm_1: u8, shrt_nm_2: u8) {
...
}
```

```rust
{
// Complex line (not just a function call, also a let statement). Full
// structure.
let (a, b) = bar(
really_long_parameter_name_1,
really_long_parameter_name_2,
shrt_nm_1,
shrt_nm_2,
);

// Long, simple function call.
waz(
really_long_parameter_name_1,
really_long_parameter_name_2,
shrt_nm_1,
shrt_nm_2,
);

// Short function call. Inline.
baz(a, b);
}
```

- Always end last item of a multi-line comma-delimited set with `,` when legal:

```rust
struct Point<T> {
x: T,
y: T, // <-- Multiline comma-delimited lists end with a trailing ,
}

// Single line comma-delimited items do not have a trailing `,`
enum Meal { Breakfast, Lunch, Dinner };
```

- Avoid trailing `;`s where unneeded.

```rust
if condition {
return 1 // <-- no ; here
}
```

- `match` arms may be either blocks or have a trailing `,` but not both.
- Blocks should not be used unnecessarily.

```rust
match meal {
Meal::Breakfast => "eggs",
Meal::Lunch => { check_diet(); recipe() },
// Meal::Dinner => { return Err("Fasting") } // WRONG
Meal::Dinner => return Err("Fasting"),
}
```

# Style

- Panickers require explicit proofs they don't trigger. Calling `unwrap` is discouraged. The
exception to this rule is test code. Avoiding panickers by restructuring code is preferred if
feasible.

```rust
let mut target_path =
self.path().expect(
"self is instance of DiskDirectory;\
DiskDirectory always returns path;\
qed"
);
```

- Unsafe code requires explicit proofs just as panickers do. When introducing unsafe code,
consider trade-offs between efficiency on one hand and reliability, maintenance costs, and
security on the other. Here is a list of questions that may help evaluating the trade-off while
preparing or reviewing a PR:
- how much more performant or compact the resulting code will be using unsafe code,
- how likely is it that invariants could be violated,
- are issues stemming from the use of unsafe code caught by existing tests/tooling,
- what are the consequences if the problems slip into production.
the-right-joyce marked this conversation as resolved.
Show resolved Hide resolved

# Manifest Formatting

> **TLDR**
> You can use the CLI tool [Zepter](https://crates.io/crates/zepter) to format the files: `zepter format features`

Rust `Cargo.toml` files need to respect certain formatting rules. All entries need to be alphabetically sorted. This makes it easier to read them and insert new entries. The exhaustive list of rules is enforced by the CI. The general format looks like this:

- The feature is written as a single line if it fits within 80 chars:
```toml
[features]
default = [ "std" ]
```

- Otherwise the feature is broken down into multiple lines with one entry per line. Each line is padded with one tab and no trailing spaces but a trailing comma.
```toml
[features]
default = [
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong",
# Comments go here as well ;)
"std",
]
```
Loading
Loading