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

Rust: Bump deps & 2021 edition, reexports, clippy #282

Merged
merged 7 commits into from
Aug 11, 2023

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented Aug 9, 2023

  • Re-export the entire geozero because it has a number of other used traits like Error, so reexporting partial geozero creates more problem than solves
  • Switch all crates to 2021 edition
  • Cleanup crate.toml - we may want to switch to a proper workspace here
  • bump all dependency versions to latest
  • Address some clippy warnings

* Re-export the entire geozero because it has a number of other used traits like Error, so reexporting partial geozero creates more problem than solves
* Switch all crates to 2021 edition
* Cleanup crate.toml - we may want to switch to a proper workspace here
* bump all dependency versions to latest
* Address some clippy warnings
```
cargo clippy --fix
cargo clippy --workspace --allow-dirty --fix --benches --tests --bins -- -A clippy::all -W clippy::uninlined_format_args
```
* rename internal `to_feature` because it breaks Rust naming expectations
* add a safety docs to `FgbReader::open_unchecked` -- please doublecheck that it is correct
* fix a lot of `.write(...)` to `write_all(...)` - because write does not guarantee how many bytes it will write
* removed a few lifetimes - clippy thinks they are not needed
* `&Vec<_>` -> `&[_]`
* Optimized `PackedRTree::build` iter
@nyurik
Copy link
Contributor Author

nyurik commented Aug 9, 2023

cc: @pka

@bjornharrtell
Copy link
Member

Thanks @nyurik, this looks good to me but if @pka is available I'll await his review before merge.

@pka
Copy link
Member

pka commented Aug 10, 2023

All good for me, many great improvements!

@nyurik
Copy link
Contributor Author

nyurik commented Aug 10, 2023

thx @pka and @bjornharrtell , feel free to merge and release. I plan a release for geozero with some breaking changes, so the next release beyond these changes may have to be coordinated later too 🤦

@pka pka requested review from pka and removed request for pka August 10, 2023 19:37
Copy link
Member

@bjornharrtell bjornharrtell left a comment

Choose a reason for hiding this comment

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

I note that this also upgrades flatbuffers to 23.5.26. I will see to it that this same version is used cross languages. It's not strictly required since the wire format is guaranteed to be compatible forever, but I still feel it's good to align this.

@bjornharrtell
Copy link
Member

@nyurik do you agree this should be squash merged?

@nyurik
Copy link
Contributor Author

nyurik commented Aug 11, 2023

Sure, I always squash merge

@bjornharrtell bjornharrtell merged commit b013ac3 into flatgeobuf:master Aug 11, 2023
9 checks passed
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.

None yet

3 participants