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

[prdoc] Update docs #3998

Merged
merged 6 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
100 changes: 55 additions & 45 deletions docs/contributor/prdoc.md
Original file line number Diff line number Diff line change
@@ -1,55 +1,31 @@
# PRDoc

## Intro

With the merge of [PR #1946](https://github.com/paritytech/polkadot-sdk/pull/1946), a new method for
documenting changes has been introduced: `prdoc`. The [prdoc repository](https://github.com/paritytech/prdoc)
contains more documentation and tooling.

The current document describes how to quickly get started authoring `PRDoc` files.
A [prdoc](https://github.com/paritytech/prdoc) is like a changelog but for a Pull Request. We use this approach to
record changes on a crate level. This information is then processed by the release team to apply the correct crate
version bumps and to generate the CHANGELOG of the next release.

## Requirements

When creating a PR, the author needs to decides with the `R0` label whether the change (PR) should
appear in the release notes or not.

Labelling a PR with `R0` means that no `PRDoc` is required.

A PR without the `R0` label **does** require a valid `PRDoc` file to be introduced in the PR.

## PRDoc how-to

A `.prdoc` file is a YAML file with a defined structure (ie JSON Schema).

For significant changes, a `.prdoc` file is mandatory and the file must meet the following
requirements:
- file named `pr_NNNN.prdoc` where `NNNN` is the PR number.
For convenience, those file can also contain a short description: `pr_NNNN_foobar.prdoc`.
- located under the [`prdoc` folder](https://github.com/paritytech/polkadot-sdk/tree/master/prdoc) of the repository
- compliant with the [JSON schema](https://json-schema.org/) defined in `prdoc/schema_user.json`

Those requirements can be fulfilled manually without any tooling but a text editor.

## Tooling

Users might find the following helpers convenient:
- Setup VSCode to be aware of the prdoc schema: see [using VSCode](https://github.com/paritytech/prdoc#using-vscode)
- Using the `prdoc` cli to:
- generate a `PRDoc` file from a [template defined in the Polkadot SDK
repo](https://github.com/paritytech/polkadot-sdk/blob/master/prdoc/.template.prdoc) simply providing a PR number
- check the validity of one or more `PRDoc` files
When creating a PR, the author needs to decide with the `R0-silent` label whether the PR has to contain a prdoc. The
`R0` label should only be placed for No-OP changes like correcting a typo in a comment or CI stuff. If unsure, ping
the [CODEOWNERS](../../.github/CODEOWNERS) for advice.

## `prdoc` cli usage
## PRDoc How-To

The `prdoc` cli documentation can be found at https://github.com/paritytech/prdoc#prdoc
A `.prdoc` file is a YAML file with a defined structure (ie JSON Schema). Please follow these steps to generate one:

tldr:
- `prdoc generate <NNNN>`
- `prdoc check -n <NNNN>`
1. Install the [`prdoc` CLI](https://github.com/paritytech/prdoc) by running `cargo install prdoc`.
1. Open a Pull Request and get the PR number.
1. Generate the file with `prdoc generate <PR_NUMBER>`. The output filename will be printed.
1. Optional: Install the `prdoc/schema_user.json` schema in your editor, for example
[VsCode](https://github.com/paritytech/prdoc?tab=readme-ov-file#schemas).
1. Edit your `.prdoc` file according to the [Audience](#pick-an-audience) and [SemVer](#record-semver-changes) sections.
1. Check your prdoc with `prdoc check -n <PR_NUMBER>`. This is optional since the CI will also check it.

where <NNNN> is the PR number.
> **Tip:** GitHub CLI and jq can be used to provide the number of your PR to generate the correct file:
> `prdoc generate $(gh pr view --json number | jq '.number') -o prdoc`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done by prdoc automatically.


## Pick an audience
## Pick An Audience

While describing a PR, the author needs to consider which audience(s) need to be addressed.
The list of valid audiences is described and documented in the JSON schema as follow:
Expand All @@ -65,7 +41,41 @@ The list of valid audiences is described and documented in the JSON schema as fo

- `Runtime User`: Anyone using the runtime. This can be a token holder or a dev writing a front end for a chain.

## Tips
If you have a change that affects multiple audiences, you can either list them all, or write multiple sections and
re-phrase the changes for each audience.

## Record SemVer Changes

All published crates that got modified need to have an entry in the `crates` section of your `PRDoc`. This entry tells
the release team how to bump the crate version prior to the next release. It is very important that this information is
correct, otherwise it could break the code of downstream teams.

The bump can either be `major`, `minor`, `patch` or `none`. The three first options are defined by
[rust-lang.org](https://doc.rust-lang.org/cargo/reference/semver.html), whereas `None` should be picked if no other
applies. The `None` option is equivalent to the `R0-silent` label, but on a crate level. Experimental and private APIs
are exempt from bumping and can be broken at any time. Please read the [Crate Section](../RELEASE.md) of the RELEASE doc
about them.

> **Note**: There is currently no CI in place to sanity check this information, but should be added soon.

### Example

For example when you modified two crates and record the changes:

```yaml
crates:
- name: frame-example
bump: major
- name: frame-example-pallet
bump: minor
```

It means that downstream code using `frame-example-pallet` is still guaranteed to work as before, while code using
`frame-example` might break.

### Dependencies

The PRDoc schema is defined in each repo and usually is quite restrictive.
You cannot simply add a new property to a `PRDoc` file unless the Schema allows it.
A crate that depends on another crate will automatically inherit its `major` bumps. This means that you do not need to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the crate inherits the major bump?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we do this as part of the release process, or @Morganamilo ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean you will need to figure out if the crate is in the public api to justify a major bump of the dependent crate?

Copy link
Member Author

@ggwpez ggwpez Apr 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"in the public api" is very difficult for a developer to see. It could also mean that some behaviour was changed, but all the API definitions stay the same. And this would need to be done transitively by the developer.

We have no way to detect this, and developers could make mistakes by not adding all transitive dependants. So for now i think we just major-bump all dependants.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up deciding it does not matter whether the crate is in the public API (though parity-publish can tell this ) as the dependency tree is so interwoven for polkadot. Consider we have a release where we have one and just one PRdoc that specifies sp-io has a major bump.

morganamilo@songbird % parity-publish prdoc ../polkadot-sdk ../polkadot-sdk/prdoc -d
sp-io (substrate/primitives/io):
    files
    Major

morganamilo@songbird % parity-publish plan --new ../polkadot-sdk --prdoc ../polkadot-sdk/prdoc 
looking up crate data, this may take a while....
434 packages changed 433 indirect
calculating order...
looking up crates...
calculating plan...
plan generated 500 packages 344 to publish

We have 3 options in terms of publishing here. We could:

  • just major bump sp-io
  • major bump sp-io and minor bump all 142 direct dependants
  • major bump sp-io and major bump all 343 indirect dependants

Option one doesn't work as we want our crates to use the new version so we have to at least also update direct dependants.

Option two is also bad in my opinion. Because semver compatible versions can be mixed and matched. We could then end up with multiple versions of sp-io in the dependency tree. Multiply that by how many crates we have and how interwoven the dependencies are. we could end up with 2?, 3?, 4?, 5? different versions of dozens of packages in theory.

Also even if this crate is not exposed in the public API of any crates. If it manages any sort of global state, then each copy of the crate would have its own global state. If crates rely on some sort of shared global state all of a sudden it's strangely not shared any more.

Option 3 is the only* clean solution to me. While it sucks to have to update so mange packages because of one change, think it the solution is to be aware of breaking changes and try to not make the often.

*if there's need I think we could possibly have a whitelist of crates where we want to do option 2, though option 3 should be the default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sp-io is also quite special because we can not have multiple different major version of this crate in our tree :D But yeah, what you write makes sense and we don't have that many other options. TLDR, do not break stuff :P

bump a crate that had a SemVer breaking change only from re-exporting another crate with a breaking change.
`minor` an `patch` bumps do not need to be inherited, since `cargo` will automatically update them to the latest
compatible version.
5 changes: 5 additions & 0 deletions prdoc/schema_user.json
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@
"const": "patch",
"title": "Patch",
"description": "A bump to the third leftmost non-zero digit of the version number."
},
{
"const": "none",
"title": "None",
"description": "This change requires no SemVer bump (e.g. change was a test)."
}
]
},
Expand Down
Loading