-
Notifications
You must be signed in to change notification settings - Fork 218
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
serde
feature added
#745
serde
feature added
#745
Conversation
Support for serde derivations in no_std. Part of: paritytech/substrate#12994
Looks like there is already a Also, can you add a CI jobs with this feature for |
Can you please elaborate more, how can we reuse it?
Good point, will do. |
Sorry, yeah I just meant reusing the same name for the feature. |
My personal feeling is that Naming was also discussed in paritytech/substrate#12994: However I do understand your point to have consistent naming in |
Oh, I see. Yeah, I didn't like Is it possible to have them alias to each other? That way people using |
Nothing better than |
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.
Looks good, not blocking on the feature name. Maybe it's fine without further changes, will let you decide.
bounded-collections/CHANGELOG.md
Outdated
## [0.1.6] - 2023-05-05 | ||
- Added `serde` feature, which can be enabled for no `std` deployments. |
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.
nit: If we are following semver here, should this technically be a minor version bump? Since we are adding functionality.
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.
Thanks, looks like we weren't very strict about semver in the past, but good to start 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.
Actually, since we're still at 0.y.z
we might have been following the common scheme where y
is reserved for breaking changes.
The spec recommends always bumping y
on new releases when x=0
, but probably we shouldn't change the strategy we were already following. Maybe we should revert? 🙈
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.
Both are fine for me. But following previous strategy makes sense. I'll revert.
This reverts commit 02f09de.
Co-authored-by: Bastian Köcher <git@kchr.de>
Support for
serde
derivations in no_std.Part of: paritytech/substrate#12994,
Required for: paritytech/polkadot-sdk#25