search

camino-rs / camino

Like Rust's std::path::Path, but UTF-8.
https://docs.rs/camino
Apache License 2.0
435 stars 16 forks source link

What's with the `serde1` and `proptest1` features? #85

Open Kyuuhachi opened 1 year ago

Kyuuhachi commented 1 year ago

What it says on the tin. Why declare separate features on top of the ones defined by the optional dependencies?

sunshowers commented 1 year ago

Basically, if there's ever a serde v2 or a proptest v2, then we don't want to release a camino v2 was my thought.

Kyuuhachi commented 1 year ago

I guess that makes sense, but it's kinda confusing when you enable the serde feature and serde integration is not enabled. Would it be possible/a good idea to include something like (untested)

#[cfg(all(feature = "serde", not(feature = "serde1")))]
fn serde_but_not_serde1() {} // provoke a compiler warning
TheZoq2 commented 10 months ago

I just ran into this too, an error like that would be nice

sunshowers commented 10 months ago

I think producing an error might break existing users (I wish I had the foresight to do this at the time I was releasing v1.0), and I don't think Cargo currently has a way to produce warnings from upstream crates.

The last alternative is to allow serde to work just like serde1 does today, and it's the approach some other crates have chosen I believe.

sunshowers commented 5 months ago

See https://github.com/camino-rs/camino/commit/4ae167f76e5c6f846a0bf3ec8687c8e270882117 -- the build script now emits warnings. This won't produce warnings by default, but will with cargo build -vv (which some folks run in CI at least).

Again, apologies about this -- it really sucks but is just very hard to change.