georust / geo

Geospatial primitives and algorithms for Rust
https://crates.io/crates/geo
Other
1.51k stars 195 forks source link

geo-types: Weaken `serde/std` and `approx/std` features #1024

Closed w-flo closed 1 year ago

w-flo commented 1 year ago

Previously, if I wanted only the std feature of geo_types, this would enable the serde and approx features, too. Change these features to be "weak dependency features", so they're only enabled (and their crates pulled in) if the corresponding optional crate is already pulled in for other reasons.


But I'm not sure if this could potentially be a breaking change? Ideally, it would be a purely internal change, but I'm not sure if it could cause issues for downstream crates / users. And I'm not sure if a CHANGES.md entry would be needed / what it should say?

michaelkirk commented 1 year ago

Good catch!

We added the "std" feature in an attempt to support no-std builds, and in the process it looks like we inadvertently pulled some existing disabled-by-default features into the default build. Is that right?

I fear we might have made the same mistake with some of our other no_std crates.

w-flo commented 1 year ago

Oh, yes! I hadn't seen that commit/PR. I believe you're right, that's where the approx and serde crates started to be pulled in by default because their corresponding features were not marked as "weak" using the "?" syntax. Now you can only drop them by disabling the std feature. The std feature itself is a great addition though.

Note that the "?" cargo syntax was stabilized in Rust 1.60, so it should be OK for geo_types (MSRV 1.63), but maybe some other georust crates use MSRV < 1.60?

I only use geo_types and gpx, and gpx does not appear to have this issue (but two other issues https://github.com/georust/gpx/pull/91 )

lnicola commented 1 year ago

bors r=frewsxcv,lnicola

bors[bot] commented 1 year ago

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.