georust / geozero

Zero-Copy reading and writing of geospatial data.
Apache License 2.0
321 stars 30 forks source link

address new clippy lints from rust-1.72 #173

Closed michaelkirk closed 9 months ago

nyurik commented 10 months ago

Why move the dep management to the create?

michaelkirk commented 10 months ago

My understanding is that workspace dependencies are useful when you have a dependency specified in multiple places. When you want to update that dependency's version, you can update the version in only the one workspace Cargo.toml rather than in multiple crates' Cargo.tomls.

But in this case, this dependency is used in only one place. It seems overly ceremonious to introduce a level of indirection.

nyurik commented 10 months ago

I have used them in many projects, and at first I also thought that, but later realized that managing two sets of deps is worse than having all versions in one place - much easier to manage updates

nyurik commented 10 months ago

Ps. Plus you never have to move a dep from the one use to shared set every time a dep starts being used elsewhere. So in short, easier mental model to do all of them the same way

pka commented 9 months ago

I don't see the point of declaring all dependencies in the top-level cargo.toml. For me it's an advantage to see which dependencies are shared (=top-level) and which dependencies are exclusive for a certain crate. But seems neither me nor @michaelkirk have a very strong opionon on that, since nobody started a discussion when you updated the crates.toml organization (which was a good thing).

michaelkirk commented 9 months ago

In case Yuri's concerns about the changes to the Cargo.toml were holding this up, I've moved things back to the shared Cargo.toml.

Yuri, in the future, could you be explicit when your concerns are blocking vs a non-blocking concern? It's helpful to know when you're pontificating about a matter of subjective taste that you think others might come to appreciate vs. when you've found something actually critical to address before moving forward. Otherwise I'm left in limbo, and you've put the burden on me to guess.

nyurik commented 9 months ago

Terribly sorry, i should have been more explicit! I would love to have cleaner single point of dep management, and i feel "mid-level" strength about it. If you have very strong opinion either way, I won't block, but if not, i do prefer it consistent for ease-of-updating and to see all dependencies in one place.