search

paritytech / parity-common

Collection of crates used in Parity projects
https://www.paritytech.io/
Apache License 2.0
282 stars 213 forks source link

`serde` feature added #745

Closed michalkucharczyk closed 1 year ago

michalkucharczyk commented 1 year ago

Support for serde derivations in no_std.

Part of: https://github.com/paritytech/substrate/issues/12994, Required for: https://github.com/paritytech/polkadot-sdk/issues/25

mrcnski commented 1 year ago

Looks like there is already a serde_no_std feature we could reuse here: https://github.com/paritytech/parity-common/pull/385

Also, can you add a CI jobs with this feature for bounded-collections?

michalkucharczyk commented 1 year ago

Looks like there is already a serde_no_std feature we could reuse here: paritytech/substrate#385

Can you please elaborate more, how can we reuse it? Do you mean renaming serde to serde_no_std?

Also, can you add a CI jobs with this feature for bounded-collections?

Good point, will do.

mrcnski commented 1 year ago

Can you please elaborate more, how can we reuse it? Do you mean renaming serde to serde_no_std?

Sorry, yeah I just meant reusing the same name for the feature.

michalkucharczyk commented 1 year ago

My personal feeling is that serde is just a good name for enabling this feature. It is just about adding serde deps with proper features.

Naming was also discussed in https://github.com/paritytech/substrate/issues/12994: serde will be used in across substrate.

However I do understand your point to have consistent naming in parity-common. Let me know what do you think?

mrcnski commented 1 year ago

Oh, I see. Yeah, I didn't like serde_no_std, but changing it would be breaking and thought we should be consistent.

Is it possible to have them alias to each other? That way people using serde_no_std can keep using it, but we move to serde in substrate.

michalkucharczyk commented 1 year ago

Nothing better than serde = [ "serde_no_std"] comes to my mind. I don't know how to deprecate feature. Found this: https://github.com/rust-lang/cargo/issues/7130, https://stackoverflow.com/questions/56917787/how-to-correctly-deprecate-a-crate-feature.