Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Release Process #2490
Release Process #2490
Changes from 27 commits
11aa7ac
5eea0df
d8b7524
d622a7c
1e5b3b0
becbb79
316579d
11b77b0
6cdfd59
35d87bf
5189616
a769906
6d61040
d9bf9ef
71d9062
4d65917
1cc2780
423d0ab
9d7bdfd
2bdd380
51322b3
2cd25d9
554b247
d11e585
26d5a8a
7de5e5e
ad657d9
7158244
bfad729
1e10077
f8dea0b
3341c6b
3702afb
dce630d
c3014c4
702118b
ae7e73c
7a4b801
6169ee8
d03a2d3
80b81b1
9328f76
e7032c5
818c013
f36af66
d01c07a
ef1f428
d11235f
09e539a
fc28e27
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The release team needs to confirm, but I think releases can take a very long time. We should check that releasing nightly is feasible, and probably only release creates with changes (incl deps). I can update https://github.com/liamaharon/cargo-workspace-version-tools to handle that.
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.
Yes i think having nightly releases right from the start is unrealistic.
It is still put in here as "how it should be", but i dont assume to see nightly releases soon.
I think the release team has a tool to only publish the changed crates, but it seems to lack automatization https://github.com/paritytech/parity-publish
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 considers only substrate/parachain focusing elements. I think most of development for Polkadot itself will happen on the stable branch directly, as we don't want to slow us down to only updating the relay chain once every 3 months. Only features that depend on new/breaking changes would need to go to master.
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.
Non-breaking changes still can be added to stable on a more frequent cadence.
Fellowship could also if they wish use the latest audited nightly version if they want to incorporate breaking changes before the next stable 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.
Yea true, did not think about this but the new process is meant to not slow us down. I guess audited nightly could work.
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 see how this could push any feature further back than 3 months (even if we avoid nightly completely) and only then in the very worst case. Given our RC development schedules tend to be of the order of years it doesn't seem all that relevant.
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.
How is this supposed to work with relay chain functionality (see above)?
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.
Virtually all relay chain functionality needs to be backported constantly?
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.
#2490 (comment) does this answer your q?
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.
Not really what would the workflow for relay chain devs look like? Like which branch would we usually base our PRs on? If I read this document right, it would be master and then we would backport to stable? And this for all Polkadot PRs?
I feel like the monorepo is biting us here a bit. From the perspective of Polkadot a more consuming perspective would make sense in my opinion. Basically we just use substrate as a normal dep, with its releases. Develop Polkadot and whenever we actually need a new substrate feature (or need security fixes) we update our dependency.
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.
Could be... but i think if we go with audited nightly then it should be very close to what we currently have.
But its not really that nice, since currently we update the Runtimes to a proper Polkadot-SDK release, in the future we would update then to a nightly 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.
Let's create an issue to track keeping the
Cargo.toml
s consistent with what's on crates.io? https://github.com/liamaharon/cargo-workspace-version-tools/ already has a script to sync them, I can create two new commands toThere 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.
Yes definitely need to keep them consistent. I will create an issue.
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.
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.
All crates should follow semver. However, we should have crates that can get merged to
stable
, even when there are breaking changes aka major version bumps.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.
@eskimor this is also what I would do to most of the relay chain crates.
We should put some custom metadata into the
Cargo.toml
telling the tooling that these crates are fine to "break". For the human we should mention this in the description of theCargo.toml
.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 think it is a prerequisite to have this done before the pr here gets merged, we can do this over time.
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.
created #3031
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.
NODE_VERSION
instead of just in the Cargo.toml?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.
What would be the point and what would be included in this? RPC? Cli? Some internal changes?
#1495 (comment)
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.
from the PoV of node operators, with node versions I would expect
If you ask node operators, I think they would appreciate semver signalling like this?
This doesn't really explain why 🤔
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 see Liam's point, that it would have been good to use SemVer to signal the behavior changes of the node.
In my understanding the reason we don't do that is that we have always targeted Polkadot node 1.0 to be the white paper. Now that it has been delivered, we have two options:
At the moment, I am leaning towards option 1. I think most operators are already used to reading changelogs and applying any CLI changes.
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.
what about having multiple PRs that modify the same crates. I guess we're supposed to bump them only once within a 3-month period right? This can be a bit cumbersome and prone to human error
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.
Yes good observation. There was some discussion on if we maybe rather use
PrDoc
to indicate SemVer bumps, but then we need additional tooling to translate that into crate bumps and it does not play well with other tools.We would probably need a CI that prevents double bumps, but i agree that it could be a point of friction.
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.
Shouldn't crate bumping only be necessary when merging to the release branch, and
master
bumps are just automated nightly releases?Do you mean to refer to the
release
branch instead ofmaster
branch here?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 was trying to describe the normal workflow of merging a standard Merge request.
Currently we never touch the
Cargo.toml
version. But in the future we would need to increment it according to SemVer. This can be amajor
,minor
orpatch
bump - depending on the change.Otherwise when would we bump these versions? We dont really "merge" to the release branch, unless its a backport.
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.
My understanding was
master
: don't need to do bumps on merge. releases will be made nightly bumping the semver with a new-nightlyYYYY-MM-DD
suffix. I don't see benefit of bumping X.Y.Z between nightly releases, because the code will have a breaking bump (from the-nightly
change) soon anyway.release
: we need to bump according to X.Y.Z semver, because we need to make new releases for the changed crates.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 how I understood it, where master branch only get nightly releases.
each crates versions like this:
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.
But then how do we keep track of these in the meantime? Yes sure we dont need to bump for the nightly releases since they get their own version number overwritten.
We would also need to bullet proof automate these version bump then. I would also like to do this with prdoc and so on, but @bkchr seems to be in favour of just bumping them directly. Note that we dont need to bump multiple times on master if there was no release so far.
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.
In my proposal, yes.
In my proposal:
release
every 3 months when the Clobber happens. SemVer respected.release
every 2 weeks for any crate touched by a backport or any crate depending on such a crate. Backport reviews make an explicit check that they do not make breaking changes. SemVer respected.release
for out-of-band security fixes which have been reviewed as non-breaking. SemVer respected.-nightly
suffix. SemVer respected.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.
The nightly is a pre-release, since it has a dash in the version name. It is completely irrelevant for any SemVer considerations and can break at any time.
Yes, but only temporarily in the CI. This bump is never actually committed.
As you said below, this could introduce major bump without breaking changes. But maybe it would still be a good tradeoff if the other way introduces too much overhead. I mainly dont like it much because it breaks
cargo update
.We dont have to "worry" about them, but we still have to mark them as breaking.
Just as note: I interpret this as meaning that unnecessary major bump should be avoided, since SemVer does not require them.
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.
Okay then lets try the native bumping without Prdoc for now.
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.
Do we need to update this to clarify manual 'crate bumping' applies to backports into
stable
notmaster
?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.
What do you mean? We would bump crates in the MR that merges into
master
.Then that version bump could get back ported to
stable
, but its version should not need any tweaking.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.
What should work? Building the binary? Sounds like bullshit to me, again CI is building the stuff all the time.
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.
Yes but there are lots of build errors that only arrive when building with
cargo publish --dry-run
.For example the location of the README or other relative paths gets messed up within the workspace, since it builds each crate in isolation.
Eventually i dont mind how the release team ensures that it all works, but i think for a start it is fine to have in here.
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.
Basically we would need to release all of the crates or not? Or the tool that checks if something changed in a crate, needs to take into consideration the dependency graph. Aka we changed something deep inside the graph and thus all crates that depend on this crate are also bumped.
Which brings up another point, we can not just do what you said in point 3. We only need to bump the version of the crates that changed. We will then also need to depend on some of the older nightly releases in crates. In the end we will end up with every crate being on a different nightly tag, depending on when it was modified the last time. I mean this works, but just highlights again that we need some tooling to ensure that we use the correct crate versions for every crate in some downstream project.
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.
AFAIK parity-publish does exactly this.
Ah crap. Did not think about this.
Yea this sounds like a nightmare. We could also create an overarching
polkadot-sdk
crate that locks in all crate versions. Otherwise some custom bumper CLI...I think the release team was building such a CLI to do the bumping in downstream repos.
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.
What is the problem with merging?
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.
Copying files is not quite the best term.. It just means nuke the working tree with whatever is the most recently audited commit in nightly, instead of trying to do merge conflict resolution or whatever.
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.
Which would prohibit us from working on stable directly for Polkadot - correct?
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.
Stable cannot have changes that were not in master first. Problem are the git conflicts and nuking is easier.
Yes
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.
you mean we'll have automation for this? Most likely the backport will have conflicts or even logic changes that don't always result in git conflicts
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.
In the trivial case I guess the back port could be made automatically. But in general it would likely be two separate (similar) PRs.