From a17097172c6e88ca3da7dd751c960212d95056a2 Mon Sep 17 00:00:00 2001 From: Alexander Popiak <alexander.popiak@parity.io> Date: Sat, 13 Jun 2020 18:55:44 +0200 Subject: [PATCH] Update contributing guide with new label policy (#6333) * mention C and M labels in contributing guide * update PR template with more specific instructions * update PR template with updated label rules and contributing guide link * update contibuting guide --- substrate/docs/CONTRIBUTING.adoc | 35 ++++++++++++++++++++----- substrate/docs/PULL_REQUEST_TEMPLATE.md | 13 ++++++--- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/substrate/docs/CONTRIBUTING.adoc b/substrate/docs/CONTRIBUTING.adoc index b573aef50d8..3dca7432c08 100644 --- a/substrate/docs/CONTRIBUTING.adoc +++ b/substrate/docs/CONTRIBUTING.adoc @@ -19,17 +19,30 @@ There are a few basic ground-rules for contributors (including the maintainer(s) == Merge Process -Merging pull requests once CI is successful: +*In General* -. A PR needs to be reviewed and approved by project maintainers unless: - - it does not alter any logic (e.g. comments, dependencies, docs), then it may be tagged https://github.com/paritytech/substrate/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+label%3AA2-insubstantial[`insubstantial`] and merged by its author once CI is complete. - - it is an urgent fix with no large change to logic, then it may be merged after a non-author contributor has approved the review once CI is complete. +A PR needs to be reviewed and approved by project maintainers unless: +- it does not alter any logic (e.g. comments, dependencies, docs), then it may be tagged https://github.com/paritytech/substrate/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+label%3AA2-insubstantial[`insubstantial`] and merged by its author once CI is complete. +- it is an urgent fix with no large change to logic, then it may be merged after a non-author contributor has approved the review once CI is complete. + +*Labels TLDR:* + +- `A-*` Pull request status. ONE REQUIRED. +- `B-*` Changelog and/or Runtime-upgrade post composition markers. ONE REQUIRED. (used by automation) +- `C-*` Release notes release-priority markers. EXACTLY ONE REQUIRED. (used by automation) +- `D-*` More general tags on the PR denoting various implications and requirements. + +*Process:* + +. Please tag each PR with exactly one `A`, `B` and `C` label at the minimum. . Once a PR is ready for review please add the https://github.com/paritytech/substrate/pulls?q=is%3Apr+is%3Aopen+label%3AA0-pleasereview[`A0-pleasereview`] label. Generally PRs should sit with this label for 48 hours in order to garner feedback. It may be merged before if all relevant parties had a look at it. . If the first review is not an approval, swap `A0-pleasereview` to any label `[A3, A7]` to indicate that the PR has received some feedback, but needs further work. For example. https://github.com/paritytech/substrate/labels/A3-inprogress[`A3-inprogress`] is a general indicator that the PR is work in progress and https://github.com/paritytech/substrate/labels/A4-gotissues[`A4-gotissues`] means that it has significant problems that need fixing. Once the work is done, change the label back to `A0-pleasereview`. You might end up swapping a few times back and forth to climb up the A label group. Once a PR is https://github.com/paritytech/substrate/labels/A8-mergeoncegreen[`A8-mergeoncegreen`], it is ready to merge. . PRs must be tagged with respect to _release notes_ with https://github.com/paritytech/substrate/labels/B0-silent[`B0-silent`] and `B1-..`. The former indicates that no changes should be mentioned in any release notes. The latter indicates that the changes should be reported in the corresponding release note -. PRs that break the external API must be tagged with https://github.com/paritytech/substrate/labels/B2-breaksapi[`B2-breaksapi`], when it changes the FRAME or consensus of running system with https://github.com/paritytech/substrate/labels/B3-breaksconsensus[`B3-breaksconsensus`] -. No PR should be merged until all reviews' comments are addressed. +. PRs that break the external API must be tagged with https://github.com/paritytech/substrate/labels/B2-breaksapi[`B2-breaksapi`], when it changes the FRAME or consensus of running system with https://github.com/paritytech/substrate/labels/B3-breaksconsensus[`B3-breaksconsensus`]. +. PRs should be labeled with their release importance via the `C1-C9`. +. PRs should be categorized into projects. +. No PR should be merged until all reviews' comments are addressed and CI is successful. *Reviewing pull requests*: @@ -49,7 +62,7 @@ When reviewing a pull request, the end-goal is to suggest useful changes to the === Updating Polkadot as well -**All pull requests will be checked agains either Polkadot master, or your provided Polkadot companion PR**. That is, If your PR changes the external APIs or interfaces used by Polkadot. If you tagged the PR with `breaksapi` or `breaksconsensus` this is most certainly the case, in all other cases check for it by running step 1 below. +**All pull requests will be checked against either Polkadot master, or your provided Polkadot companion PR**. That is, If your PR changes the external APIs or interfaces used by Polkadot. If you tagged the PR with `breaksapi` or `breaksconsensus` this is most certainly the case, in all other cases check for it by running step 1 below. To create a Polkadot companion PR: @@ -69,6 +82,14 @@ As there might be multiple pending PRs that might conflict with one another, a) We use https://github.com/paritytech/substrate/labels[labels] to manage PRs and issues and communicate state of a PR. Please familiarize yourself with them. Furthermore we are organizing issues in https://github.com/paritytech/substrate/milestones[milestones]. Best way to get started is to a pick a ticket from the current milestone tagged https://github.com/paritytech/substrate/issues?q=is%3Aissue+is%3Aopen+label%3AQ2-easy[`easy`] or https://github.com/paritytech/substrate/issues?q=is%3Aissue+is%3Aopen+label%3AQ3-medium[`medium`] and get going or https://github.com/paritytech/substrate/issues?q=is%3Aissue+is%3Aopen+label%3AX1-mentor[`mentor`] and get in contact with the mentor offering their support on that larger task. +== Issues +Please label issues with the following labels: + +. `I-*` Issue severity and type. EXACTLY ONE REQUIRED. +. `P-*` Issue priority. AT MOST ONE ALLOWED. +. `Q-*` Issue difficulty. AT MOST ONE ALLOWED. +. `Z-*` More general tags on the issue, denoting context and resolution. + == Releases Declaring formal releases remains the prerogative of the project maintainer(s). diff --git a/substrate/docs/PULL_REQUEST_TEMPLATE.md b/substrate/docs/PULL_REQUEST_TEMPLATE.md index fa2c15e6c02..8ca6ba9b01f 100644 --- a/substrate/docs/PULL_REQUEST_TEMPLATE.md +++ b/substrate/docs/PULL_REQUEST_TEMPLATE.md @@ -6,17 +6,24 @@ Before you submitting, please check that: - What does it do? - What important points reviewers should know? - Is there something left for follow-up PRs? -- [ ] You labeled the PR with appropriate labels if you have permissions to do so. +- [ ] You labeled the PR appropriately if you have permissions to do so: + - [ ] `A*` for PR status (**one required**) + - [ ] `B*` for changelog (**one required**) + - [ ] `C*` for release notes (**exactly one required**) + - [ ] `D*` for various implications/requirements + - [ ] Github's project assignment - [ ] You mentioned a related issue if this PR related to it, e.g. `Fixes #228` or `Related #1337`. - [ ] You asked any particular reviewers to review. If you aren't sure, start with GH suggestions. -- [ ] Your PR adheres [the style guide](https://wiki.parity.io/Substrate-Style-Guide) - - In particular, mind the maximal line length. +- [ ] Your PR adheres to [the style guide](https://wiki.parity.io/Substrate-Style-Guide) + - In particular, mind the maximal line length of 100 (120 in exceptional circumstances). - There is no commented code checked in unless necessary. - Any panickers have a proof or removed. - [ ] You bumped the runtime version if there are breaking changes in the **runtime**. - [ ] You updated any rustdocs which may have changed - [ ] Has the PR altered the external API or interfaces used by Polkadot? Do you have the corresponding Polkadot PR ready? +Refer to [the contributing guide](https://github.com/paritytech/substrate/blob/master/docs/CONTRIBUTING.adoc) for details. + After you've read this notice feel free to remove it. Thank you! -- GitLab