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

Relax deps #19

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Relax deps #19

merged 2 commits into from
Oct 2, 2024

Conversation

gadomski
Copy link
Collaborator

@gadomski gadomski commented Oct 1, 2024

What I am changing

  • Relax the dependencies and switch to geozero v0.14 from a fork
  • Update the docs link to the released spec

This shades on personal preference, but as a library-first crate, I prefer to relax the version requirements and trust SemVer a bit more. Cargo.lock still lets CI and others build exactly like we build, but it lets downstreams have a bit more flexibility.

It also reduces the maintenance burden of keeping deps up to date.

@gadomski gadomski requested review from bitner and kylebarron and removed request for bitner October 2, 2024 11:40
@gadomski gadomski self-assigned this Oct 2, 2024
@kylebarron
Copy link
Member

This shades on personal preference, but as a library-first crate, I prefer to relax the version requirements and trust SemVer a bit more. Cargo.lock still lets CI and others build exactly like we build, but it lets downstreams have a bit more flexibility.

The advice I've heard before is to set the versions in Cargo.toml to the earliest version with which you've tested. The rationale being that you don't know if there are bug fixes you rely on that were released in 1.0.5 that didn't exist in 1.0.0. And thus if you set package = "1" in your Cargo.toml, you allow users to access incompatible versions.

Cargo.toml Outdated Show resolved Hide resolved
@gadomski
Copy link
Collaborator Author

gadomski commented Oct 2, 2024

The advice I've heard before is to set the versions in Cargo.toml to the earliest version with which you've tested.

I think that makes sense except that "I made code in my library that depends on a PATCH release" feels unlikely, I guess? A PATCH should (usually) be transparent to a downstream, which is why "1.2" feels more like the actual requirement to me. But I don't really have a strong preference — I just find that many dependency checking tools throw a lot of noise when we go all the way to PATCH. But maybe that's a tool problem to solve, not something to tweak in our dependences?

Basically, don't have a strong opinion, other than that I agree that "1" is probably bad 👍🏼.

@gadomski gadomski merged commit 62d3644 into main Oct 2, 2024
1 check passed
@gadomski gadomski deleted the relax-deps branch October 2, 2024 13:55
@kylebarron
Copy link
Member

kylebarron commented Oct 2, 2024

I think that makes sense except that "I made code in my library that depends on a PATCH release" feels unlikely, I guess?

That doesn't seem that unlikely if you rely on a bug fix in a patch release? It doesn't always matter to pin to a min patch version in semver, but sometimes it really does matter, and you can't tell which case is which just by looking at a Cargo.toml in isolation.

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.

2 participants