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 MSRV: 1.66.0 -> 1.70.0 #483

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

matthiasbeyer
Copy link
Collaborator

No description provided.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
@ijackson
Copy link
Contributor

I think this is overly high. In the Arti project we are using config-rs and our MSRV is currently 1.65.

Perhaps the problem is testing with too-new versions of dependencies? In derive-adhoc I maintain a separate Cargo.lock.minimal which allows demonstrating that the MSRV works without being exposed to MSRV bumps in dependencies.

@ijackson
Copy link
Contributor

Having said that, I don't think there is any problem with you changing the documentation to say you don't support earlier than 1.70. That doesn't stop us using it with 1.65, and it works for us there. But please don't add a rust-version to Cargo.toml, since that would make it actually break.

@matthiasbeyer
Copy link
Collaborator Author

I think this is overly high.

You need to consider that this is for the next release, which (as far as I would say) isn't due to in the next few days, but rather maybe at the end of the year (read: after two more rust versions). So right now this means we're working on the last three rust versions, but when the next version of this crate is published, it is more like the last five rust versions!

@matthiasbeyer
Copy link
Collaborator Author

But please don't add a rust-version to Cargo.toml, since that would make it actually break.

It's not my intention to do so 😆

@ijackson
Copy link
Contributor

If you like, I could try making an MR that changes the MSRV test: IMO it should downgrade the dependencies and test with 1.66 (or 1.65 or even earlier - I bet your docs are overly conservative).

@ijackson
Copy link
Contributor

(but you should probably merge this first to unblock all the stuck MRs)

@matthiasbeyer
Copy link
Collaborator Author

matthiasbeyer commented Oct 23, 2023

Please correct me if I am wrong @ijackson - this is okay with you now, isn't it? Because it only affects the next minor version, not the patch release we're planing for the 0.13.x branch of things, right?

@ijackson
Copy link
Contributor

It's OK either way.

@matthiasbeyer matthiasbeyer merged commit e95b9dd into mehcode:master Oct 23, 2023
15 checks passed
@matthiasbeyer matthiasbeyer deleted the update-msrv branch October 23, 2023 16:22
@polarathene
Copy link
Collaborator

polarathene commented Oct 23, 2023

It's not my intention to do so 😆

I did though 😅 #478

cargo-msrv should identify the actual MSRV for the crate via dependencies? Then the version should be set in Cargo.toml with rust-version?


@ijackson is the concern due to dependencies incorrectly setting a rust-version that is too high? I've seen that done in config-rs in the past I think stating a higher MSRV was necessary for dev-dependencies, but that'd be for CI / tests AFAIK.

I know that warp was version pinned in the past due to an issue with its MSRV being higher (1.51 for const generics) than comfortable for config-rs at the time, and this was a dev-dependency.

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