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

Upgrade rust library dependencies and bump version #422

Merged
merged 3 commits into from
Dec 29, 2020

Conversation

fintelia
Copy link
Contributor

The Rust library depends on rather outdated versions of geo and rand so this PR upgrades to the most recent ones. To be usable with the wider ecosystem, a new version will have to be pushed to crates.io so I've bumped the library version accordingly.

@google-cla google-cla bot added the cla: yes label Dec 24, 2020
Copy link
Contributor

@bilst bilst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! I'm very new to rust so hope these comments make sense :)

let lat = rng.gen_range(-90.0, 90.0);
let lng = rng.gen_range(-180.0, 180.0);
let mut len = rng.gen_range(2, 15);
let lat = rng.gen_range(-90.0..90.0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the prevailing convention with gen_range is to pass in the interval endpoints rather than a range. Unless this is backwards-compatible and there's some recommendation to change style in new usages, mind leaving as it was?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually more forceful than just a style recommendation. The underlying rand crate changed the function declaration so only the new version will even compile.

@@ -9,7 +9,7 @@ keywords = ["geography", "geospatial", "gis", "gps", "olc"]
exclude = ["rust.iml"]

[dependencies]
geo = "^0.4.3"
geo = "0.16.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is effectively dropping support for versions < 0.16.0, right? Can you add some description of why that's desirable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust uses a slightly weird variant of semver so versions 0.4.x, 0.5.x, .... 0.16.x, 1.x.y, 2.x.y are all mutually incompatible (you can see more details here). The net result is that previously the library only supported versions 0.4.x and after this change will support only 0.16.x.

@bilst bilst merged commit d47d9f9 into google:master Dec 29, 2020
@fintelia
Copy link
Contributor Author

Now I believe @JamesFysh is the only one that can publish a new version to crates.io.

(Side note: it might make sense to add additional people or a GitHub team as owners so you don't lose access.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants