Versioned feature to support rstar 0.9 #682
Conversation
There was a problem hiding this comment.
This approach makes sense to me. Thanks for workshopping with us @rmanoka.
| @@ -22,6 +23,7 @@ serde = { version = "1", optional = true, features = ["derive"] } | |||
| # rstar integration relies on the optional approx crate, but implicit features cannot yet enable other features. | |||
| # See: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#namespaced-features | |||
| rstar = { version = "0.8", optional = true } | |||
There was a problem hiding this comment.
so, if/when we eventually make a major update to geo-types, we'd presumably drop the older rstars?
There was a problem hiding this comment.
Yes, we can drop old versions based on some agreeable policy; simplest would be to drop all but latest.
Another question is we adopt a uniform "use-{crate}-{version}" name for the feature or if there's going to be one without version. I think the uniform naming makes more sense, but don't have a strong opinion.
There was a problem hiding this comment.
I'd say uniform. I didn't add a comment about it because it would imply a breaking change in the next release, which I assume you wanted to avoid.
There was a problem hiding this comment.
Makes sense. Will add some docs so we adopt a policy at the next breaking release.
|
Thanks for the approvals. I'll go ahead and refactor the rstar tests to work with either feature, and merge this if all tests pass. Any thoughts on CI? I wonder if too many versions would lead to too many combinations here. Not an issue at the moment I think. Edit: typos |
|
I'd enable all the versions, but not test the combinations. |
|
bors try |
tryBuild succeeded: |
|
bors r=@lnicola,@michaelkirk |
|
Build succeeded: |
CHANGES.mdif knowledge of this change could be valuable to users.Another approach to solve resolve #660 . It seems this approach is not uncommon: eg. https://docs.rs/tokio-postgres/0.7.5/tokio_postgres/#features . It does involve a bit of boiler-plate macros though :(