-
Notifications
You must be signed in to change notification settings - Fork 681
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
Write some docs about the release process #2366
Conversation
This is only a first step and not the end. Later it is probably required that we come up with a more detailed document. However, this should be a good start for an overview.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly very clear!
Declaring formal releases remains the prerogative of the project maintainer(s). | ||
## Release Process | ||
|
||
We are aiming for a **two week** release process of the `polkadot-sdk` repository. The output of a release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth mentioning/explaining the merge window? I.e. just because a PR got merged before a release, doesn't mean it made it into the release.
So, the general approach is that developers are required to bump the versions in | ||
all crates that they are changing. If there is a `major` (or `minor` on `<1.0.0`) | ||
crate version bump, it is important to also bump any crate that depends on this | ||
bumped version crate and re-export it. However, if the re-export is done in a | ||
`__private` module (that makes clear that it is internal api) the `major`/`minor` | ||
version bump doesn't ripple and only requires a `minor`/`patch` bump. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused by this paragraph. Can you please explain the re-exporting? What is the __private
module (is this just some example name?) Why do we need to bump a crate version if only the internal API changed, is it just because our tooling requires it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a public re-export for macros. This is a known pattern to make people on the outside aware that they should not use this module. However, from a pure SemVer perspective, this would be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I've never seen that pattern. I grepped and the __private
looks like it's only used in substrate. But there is nothing substrate-specific about this doc or this "Crates" section, so it should be clarified. Also, I still don't understand this:
the
major
/minor
version bump doesn't ripple and only requires aminor
/patch
bump.
But, this paragraph should just be rewritten, and ideally should explain the "why". For sure it is very hard to understand, especially for new contributors.
For the test networks we only increment the `spec_version`. The spec version is also following | ||
the node version. So, `10020` is for example the node release `1.2.0`. This versioning has no | ||
further meaning and is only done to map from an on chain `spec_version` easily to the | ||
release in this repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have already not followed this. Rococo and Westend Relays are on 103000
(with 1.3.0
release) and the system paras are on 1_003_000
(using the Fellowship "style"). Switching to this would mean decreasing the version. Can we just use the Fellowship style for consistency, M_mmm_ppp
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't care. We can do this.
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
[`cargo-semver-checks`](https://github.com/obi1kenobi/cargo-semver-checks). While | ||
the tool isn't perfect, it should help to remind the developer of checking the SemVer | ||
compatibility of its changes. In general there is not any guarantee to downstream | ||
that there isn't a breaking in between and thus a `major` (or `minor` on `<1.0.0`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that there isn't a breaking in between and thus a `major` (or `minor` on `<1.0.0`) | |
that there isn't a breaking change in between and thus a `major` (or `minor` on `<1.0.0`) |
Closing for now. Will come back :P |
This is only a first step and not the end. Later it is probably required that we come up with a more detailed document. However, this should be a good start for an overview.
The text is explaining the "goal" and not what we currently have. The things about the required CI tooling changes and some other small improvements are tracked here: #2365