chore: bump development spec_version to 1.8.0 #421
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wischli
approved these changes
Oct 13, 2022
Contributor
wischli
left a comment
wischli
left a comment
There was a problem hiding this comment.
LGTM and I support your proposal. If the current imposes issues on the SDK, we need to change that.
weichweich
approved these changes
Oct 17, 2022
ntn-x2
force-pushed
the
aa/bump-spec-version
branch
from
e5a5073 to
5dcaee8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Right now, it is very very very hard for the SDK to try to make sense of how to encode and decode stuff to and from the blockchain. This is due to the fact that we have same runtime, same
spec_version, sametransaction_version, and sameeverything_elseboth in ourmasterbranch and in ourdevelopbranch.This normally would not be an issue, but right now we have a big breaking change between the two, where
masteronly deals withAccountId32stuff, whiledevelopdeals withLinkableAccountId.Types in the SDK can be cleverly defined as starting from a given spec_version number, hence I am proposing the following:
always make sure that the
spec_versionnumber ondevelopis higher than thespec_versionnumber onmasterand any released binaries.This also follows the release flow of the SDK, since anything that has not been released in a version x, will be released not earlier than the next version y. It also assumes that patch releases will be pushed directly to
master, and then merged back intodevelop. There might be other solutions, and I don't have strong preference as long as it makes it easier for the SDK to decorate and augment types. So here I am suggesting to always merge back frommasterintodevelopand to bump thespec_versionondevelopto the next minor version. In this case, I am proposing to bump from 1.7.4 to 1.8.0. It now becomes easier for the SDK to say that versions until 1.7.4 do not contain said breaking change. In case we would switch to a release branch model, I am proposingmasterto always have aspec_versionhighest than the latest released version, following the same logic.Right now we are in an exceptional situation where we have this breaking change hanging in the codebase that is requiring the SDK to implement few hacks here and there to make it work, since we have to rely on type definitions for our RPC stuff. But if we make sure that
developis always ahead ofmaster(as it should logically be), then this is easier to handle in the SDK, especially now that the type definitions have been merged inside the SDK codebase, making it very easy to spot if there is any breaking changes.