[prdoc] Update docs #3998
[prdoc] Update docs #3998
Conversation
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
docs/contributor/prdoc.md
Outdated
| 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 downstream teams' code. | ||
|
|
||
| The bump can either be `major`, `minor` or `patch`. There are exceptions for experimental and private API. These are |
There was a problem hiding this comment.
We thought about adding a none option for tests/typos etc. Do you think that is useful @bkchr?
I was convinced in the past, but now i think we can also just do it idiomatically and do patch. I think crates like serde also do patch bumps for typos.
There was a problem hiding this comment.
patch for changing a test file or whatever seems overkill to me. but happy to do either.
There was a problem hiding this comment.
None is a sensible option. It all depends on if we want to add the small burden where devs have to think about it, or just be slightly wasteful and push patch releases where we don't really need to. The cost of patch releases are effectively 0 so I see either way as fine though 'None' would be technically more correct.
There was a problem hiding this comment.
I think crates like serde also do patch bumps for typos.
Patch for typos in the docs makes sense, patch for adding/changing a test not that much ;)
As Morgan is saying None being the technical correct solution, but patch also works. As long as you explain this, choose whatever you think is the best.
There was a problem hiding this comment.
Okay since we have the R0-silent, we can use the none as R0-silent label on a per-crate basis.
Added it now.
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
|
|
||
| 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` |
There was a problem hiding this comment.
This should be done by prdoc automatically.
|
|
||
| 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 |
There was a problem hiding this comment.
How does the crate inherits the major bump?
There was a problem hiding this comment.
I think we do this as part of the release process, or @Morganamilo ?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
"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.
There was a problem hiding this comment.
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-ioand minor bump all 142 direct dependants - major bump
sp-ioand 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.
There was a problem hiding this comment.
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
Updating the prdoc doc file to be a bit more useful for new contributors and adding a SemVer section.