search

mehcode / config-rs

⚙️ Layered configuration system for Rust applications (with strong support for 12-factor applications).
Apache License 2.0
2.43k stars 206 forks source link

Update MSRV: 1.66.0 -> 1.70.0 #483

Closed matthiasbeyer closed 8 months ago

ijackson commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 :laughing:

ijackson commented 8 months ago

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 commented 8 months ago

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

matthiasbeyer commented 8 months ago

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 commented 8 months ago

It's OK either way.

polarathene commented 8 months ago

It's not my intention to do so 😆

I did though 😅 https://github.com/mehcode/config-rs/pull/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.