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

Update some obvious crate deps #428

Closed

Conversation

Firstyear
Copy link
Contributor

This updates num_derive and bitfield. Next I want to update picky-asn but that will likely need a seperate PR.

This updates num_derive and bitfield. Next I want to update picky-asn
but that will likely need a seperate PR.
@Firstyear
Copy link
Contributor Author

For future, you can use "cargo outdated" to help with finding deps like this that need updates :)

@Superhepper
Copy link
Collaborator

Thank you for the PR.

You are indeed correct that the crate suffers from some outdated depedencies. Though that is probably due to the fact that we have a MSRV (Minimum Supported Rust Version) and when you update some dependencies it no longer work with older rust versions.

@Firstyear
Copy link
Contributor Author

Ahhh that would explain it. I'm sorry I didn't notice the MSRV.

Even with that we should be able to update most of these I think :)

@ionut-arm
Copy link
Member

Though that is probably due to the fact that we have a MSRV (Minimum Supported Rust Version) and when you update some dependencies it no longer work with older rust versions.

Which begs the question - what Rust version should we tie MSRV to? We're currently updating Parsec to 1.66 after a review of what the Linux distros we build for package, maybe we can do the same here.

@Firstyear
Copy link
Contributor Author

I think I'm biased on this topic, since I'm the maintainer of rust for OpenSUSE and SUSE, and we follow upstream release cadence so we don't have an issue with MSRV being more aggressive.

Generally the rust community is very fast moving wrt to rust toolchain upgrades, but you will hit slower moving targets especially with fedora/centos/debian in this regard.

I would say you should consider the MSRV being tied to your release support policy. If you released say "today" and the release is supported for 6 months, then you set the MSRV to 1.70 or something around a similar time as "now". Then when you release in the future you bump the MSRV to what's out then.

But of course, this may affect different downstream consumers more or less. There isn't a perfect answer here.

As well, you only need to support an MSRV for an older toolchain if something on that distro is actually also shipping this library too - else it's just speculation.

Hope these thoughts give you some ideas. :)

@Superhepper
Copy link
Collaborator

Superhepper commented Aug 31, 2023

For me it is not an issue regarding if we should update our MSRV but rather an issue regarding how this should be communicated. Because it have happened that crates have updated their MSRV and only bumped the minor or patch version of the crate and then Cargo happily just automatically uses the later versions of the crates and things breaks. So for me a MSRV update requires a "Major Version" bump. In order to avoid the risk of breaking something for some one else.

Ideally we would take the "main" branch as it is now bump the MSRV to suitable version and make 8.0.0 release. Though I release it may contain so many updates that it would be hard to adapt existing code to it.

@Firstyear
Copy link
Contributor Author

I think a major version bump on MSRV change is fair :)

@ionut-arm
Copy link
Member

So, it seems there isn't yet a full consensus on what the strategy should be for semver vs MSRV, but the tentative recommendation is to allow minor version releases to increase MSRV (but not patch releases).

I agree about the main branch release, though. I think v8 is due.

@Firstyear
Copy link
Contributor Author

So, it seems there isn't yet a full consensus on what the strategy should be for semver vs MSRV, but the tentative recommendation is to allow minor version releases to increase MSRV (but not patch releases).

I agree about the main branch release, though. I think v8 is due.

I would be okay with this :)

Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

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

I'm very much OK with this change 👍 We can merge it as soon as the MSRV PR goes through.

@Firstyear
Copy link
Contributor Author

Sounds good, I'll wait for the MSRV change then I'll rebase and update this PR.

@tgonzalezorlandoarm
Copy link
Member

MSRV update PR: #445

@Superhepper
Copy link
Collaborator

@Firstyear You should try ton rebase this PR on the HEAD of main and see if it is possible to merge it.

@Firstyear
Copy link
Contributor Author

Yep, I was thinking to just close this pr and start fresh :)

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.

5 participants