-
Notifications
You must be signed in to change notification settings - Fork 51
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
Update serde to 1.0 and remove rustc-serialize #24
Conversation
[dependencies.serde] | ||
optional = true | ||
version = ">= 0.7.0, < 0.9.0" | ||
version = "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.
Does it still work if you add default-features = false
?
I think we need that for proper no-std support.
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.
It does. serde's Cargo.toml
states that it's default std
feature is for implementations of some std
types. That doesn't touch our trait impls.
Should our std feature also activate serde's? I don't think we need to, because we don't depend on it ourselves. I don't know if someone can depend purely transitively on no_std serde but then also need to serialize std types.
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.
Right, I don't think we need to enable serde/std at all here. Features are additive, so if something else enables serde/std, like num-bigint, then serde will have std overall.
I amended the |
Thanks! bors r+ |
Build succeeded |
Fixes #9