Skip to content

Conversation

@kylebarron
Copy link
Member

@kylebarron kylebarron commented May 9, 2025

  • 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 ran cargo fmt

This seems to work fine on the read side, but the write side is fully based on geo-traits now, so it's not clear how to make it optional. If geo-traits is required, then how do we choose which version to call from write_wkt?

Ref #148 (comment)

@kylebarron kylebarron mentioned this pull request May 9, 2025
3 tasks
@kylebarron kylebarron requested a review from michaelkirk May 9, 2025 15:58
@michaelkirk
Copy link
Member

If geo-traits is required, then how do we choose which version to call from write_wkt?

I think you'd need a version of the functions for each version of the trait you're integrating with.

Maybe something like:

mod to_wkt {
    #[cfg(feature=geo_traits_0_2)]
    mod  geo_traits_0_2_to_wkt;
    #[cfg(feature=geo_traits_0_2)]
    pub use geo_traits_0_2_to_wkt::*;

    #[cfg(feature=geo_traits_0_3)]
    mod  geo_traits_0_3_to_wkt;
    #[cfg(feature=geo_traits_0_3)]
    pub use geo_traits_0_3_to_wkt::*;
 }

Note this violates the convention that features are additive, and seems like it still might be a breaking change?
To be truly non-breaking, I think I'd actually expect something like

mod to_wkt {
    mod  geo_traits_0_2_to_wkt;
    pub use geo_traits_0_2_to_wkt::*;

    #[cfg(feature=geo_traits_0_3)]
    mod  geo_traits_0_3_to_wkt;
 }

And then the caller would have to explicitly call to_wkt::geo_traits_0_3_to_wkt::write_geometry

Like I said, given how much work this is, it feels mostly academic at this point.

@kylebarron
Copy link
Member Author

kylebarron commented May 12, 2025

Like I said, given how much work this is, it feels mostly academic at this point.

What do you think is the right path forward here?

For my own purposes, one path forward is a wkb release with geo-traits 0.2, waiting for a wkb release with geo-traits 0.3, and then we can wait a similar amount of time here for a new release with geo-traits 0.3. All reading and writing in the wkb crate is very integrated into geo-traits, and so there's no way to make that an optional dependency there.

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.

3 participants