djkoloski / rust_serialization_benchmark

Benchmarks for rust serialization frameworks
514 stars 48 forks source link

Major changes #35

Closed arnabanimesh closed 1 year ago

arnabanimesh commented 1 year ago
  1. Update dependencies and make changes accordingly
  2. Set PROTOC path in Windows at build time if not set already
  3. Update flatc and protoc executables to the latest versions
  4. Prevent flatbuffers related unused imports warnings from showing up
arnabanimesh commented 1 year ago

Also removed alkahest nightly feature comment from Cargo.toml, as it is still required in the latest rust compilers.

arnabanimesh commented 1 year ago

Formatted the modified files using cargo fmt too

djkoloski commented 1 year ago

It looks like there's a lot of noise in Cargo.toml from whitespace changes, could you fix those? Spaces inside curly braces is standard practice for manifests: https://github.com/rust-lang/cargo/blob/master/Cargo.toml

Did you format the auto-generated *.rs files from protoc and flatc? Those shouldn't be formatted but we can't opt out in a rustfmt.toml yet because the features aren't stable.

The version bumps look good and I appreciate the fix for PROTOC on windows. Eventually we need a better system for updating the windows executables, but for now it works fine. I'll make sure the binaries are unchanged from the versions published on GitHub.

arnabanimesh commented 1 year ago

Made the necessary modifications to Cargo.toml

I did not run cargo fmt in my project. I just have Format on save turned on in vscode. I did not format the auto generated files, these are created automatically due to the new version of the flatbuffers crate. Not to mention, it is simply not possible because rust-analyzer continues to overwrite those autogenerated files every time I tried to change.

arnabanimesh commented 1 year ago

I have updated the workflow file. Now it should work.

djkoloski commented 1 year ago

Thanks for the PR!

arnabanimesh commented 1 year ago

Line 27 of bench_rkyv.rs can be removed. I don't know why Rust compiler was showing error back then.