parity-common icon indicating copy to clipboard operation
parity-common copied to clipboard

fix: move impl-codec/std to `default` instead of `std`

Open gakonst opened this issue 2 years ago • 6 comments

Otherwise we're unable to turn on the std feature without also pulling in all of the Parity codec dependencies which are unnecessary in non-substrate related usecases.

gakonst avatar Mar 01 '22 10:03 gakonst

Hmm, this is interesting solution to #475, but ultimately, we want https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#weak-dependency-features feature, which is going to be stabilized in rust 1.60 (coming soon: https://www.whatrustisit.com/).

That being said, I'm open to merging this PR, but need to think whether this is a breaking change.

ordian avatar Mar 01 '22 10:03 ordian

Thanks for the quick response!

I am not sure how this is handled downstream, but yeah happy to help push this over the line / help with any breaking changes.

Trying to trim deps and both this PR and the one opened by Artem in #625 would be very helpful!

gakonst avatar Mar 01 '22 10:03 gakonst

FWIW it does feel that even if it's a breaking change, should be a simple one to resolve.

gakonst avatar Mar 01 '22 10:03 gakonst

FWIW it does feel that even if it's a breaking change, should be a simple one to resolve.

I wish it was easy, but unfortunately it's not (e.g. #623). If you can wait for 1.60 rustc release, I would prefer to implement this properly with the weak dependency feature. I mean we will use it anyway, but it's a matter of doing 1 breaking release now and 1 later or only 1 later.

ordian avatar Mar 01 '22 11:03 ordian

Sounds good. How does the weak dependency feature help solve this btw?

gakonst avatar Mar 01 '22 11:03 gakonst

Take a look at https://doc.rust-lang.org/nightly/cargo/reference/features.html#dependency-features

It will be impl-codec?/std instead of impl-codec/std. Which means enable std feature of impl-codec but only if impl-codec is also enabled.

ordian avatar Mar 01 '22 11:03 ordian