Skip to content
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

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

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

w-flo
Copy link
Contributor

@w-flo w-flo commented Jun 26, 2023

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.


  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

I'm always eager to optimize my build process, and this would help with that :-)

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?

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.
@michaelkirk
Copy link
Member

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
Copy link
Contributor Author

w-flo commented Jun 26, 2023

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 georust/gpx#91 )

@lnicola
Copy link
Member

lnicola commented Jun 27, 2023

bors r=frewsxcv,lnicola

@bors
Copy link
Contributor

bors bot commented Jun 27, 2023

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.

@bors bors bot merged commit 79c23ea into georust:main Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants