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.

Sign up for GitHub

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

Jump to bottom

chore: post v0.10.34 release cleanup #1595

Merged
merged 2 commits into from
Oct 26, 2023
Merged

chore: post v0.10.34 release cleanup #1595

merged 2 commits into from
Oct 26, 2023

Conversation

wischli
Copy link
Contributor

@wischli wischli commented Oct 17, 2023

Description

  • Bumps spec_version for chains which were upgraded (Altair was not) by one
  • Removes executed migrations

Checklist:

  • I have added Rust doc comments to structs, enums, traits and functions
  • I have made corresponding changes to the documentation
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works

@wischli wischli added D0-ready Pull request can be merged without special precaution and notification. I9-release A specific release. I11-cleaning No mandatory issue that leave the repo more readable/organized labels Oct 17, 2023
@wischli wischli self-assigned this Oct 17, 2023
@mustermeiszer
Copy link
Collaborator

I thought we bumb the spec only at the end of the release process, has this changed?

@wischli
Copy link
Contributor Author

wischli commented Oct 17, 2023

I thought we bumb the spec only at the end of the release process, has this changed?

Doing this here was a result of a slack discussion I could not find quickly.

IMO, it makes sense to bump the version right after a release for two reasons:

  1. We definitely need to remove past migrations inside Upgrade$runtime$spec_version. It feels natural to replace this with Upgrade$runtime$spec_version+1. However, the Upgrade... tuple spec version should match the one of the runtime IMO.
  2. In case we need to update testnets before preparing the next release, we could do this at any point without accounting for it in some PR.

However, I have no strong opinion here. WDYT?

lemunozm
lemunozm approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@lemunozm lemunozm left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this kind of PRs!

runtime/development/src/lib.rs
@@ -140,7 +140,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("centrifuge-devel"),
impl_name: create_runtime_str!("centrifuge-devel"),
authoring_version: 1,
spec_version: 1032,
spec_version: 1033,
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why bumping 2 numbers?

@mustermeiszer
Copy link
Collaborator

However, I have no strong opinion here. WDYT?

No, I am fine with that too. Was first wondering whether it incures any danger of upgrading without notice, but shouldn't be.

@wischli wischli enabled auto-merge (squash) October 26, 2023 14:27
@wischli wischli merged commit 31fdef8 into main Oct 26, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lemunozm lemunozm lemunozm approved these changes

@mustermeiszer mustermeiszer Awaiting requested review from mustermeiszer

@NunoAlexandre NunoAlexandre Awaiting requested review from NunoAlexandre

@cdamian cdamian Awaiting requested review from cdamian

Assignees

@wischli wischli

Labels
D0-ready Pull request can be merged without special precaution and notification. I9-release A specific release. I11-cleaning No mandatory issue that leave the repo more readable/organized
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wischli @mustermeiszer @lemunozm