Require minimum Rust 1.56

Enet4 commented 1 year ago

As discussed in #379, this PR does step 1: bump the hard minimum supported version of Rust to 1.56.

Summary

change rust-toolchain accordingly
remove Cargo features 1_34 and 1_46
simplify feature gated code
update guide and documentation to no longer mention existing features
mark Location as non_exhaustive, remove _other field
inline async_body function, remove report/no_async and report/yes_async
add rust-version to manifest
change CI min version test task to use Rust 1.56
remove v1.34 compatibility tests

netlify[bot] commented 1 year ago

Deploy Preview for shepmaster-snafu ready!

Name	Link
Latest commit	8b22564bda9d31febca569810f7723af34197628
Latest deploy log	https://app.netlify.com/sites/shepmaster-snafu/deploys/64dfb4412a6aff0008940689
Deploy Preview	https://deploy-preview-381--shepmaster-snafu.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Enet4 commented 1 year ago

Just replying to some of the feedback provided before the PR:

We probably need to grep for (variations on) 1.34 / 1.39 / 1.46 to find places we talk about them in prose.

My greps yield no more results in need of updates.

Why did snafu-derive get a default = [] feature?

My mistake, I probably kept it in while I was experimenting with making the rust_v* features enabled by default. It's removed now.

The yes_async::async_body module can probably be inlined.

IIUIC this will only be the case once we require a minimum version of Rust 1.61.

fn track_caller() can probably be inlined as well

:+1: Done now.

By the way, should rust_1_61 be enabled by default? I noticed that the compatibility page claims so, but that does not appear to be the case.

shepmaster commented 1 year ago

Would you mind pulling Update compile_error test snapshots out into its own PR? It's not really about updating to 1.56, just that the current stable output has changed. (also I want it for #383 😇)

Maybe also rustfmt src/lib.rs if that's more drift from stable.

shepmaster commented 1 year ago

IIUIC this will only be the case once we require a minimum version of Rust 1.61.

Hmm, I thought it was because Rust 1.34 didn't know the async keyword (only 1.39+). That file also has a switch on 1.61, but that shouldn't be relevant for merging the file.

By the way, should rust_1_61 be enabled by default? I noticed that the compatibility page claims so, but that does not appear to be the case.

Hmm, there's two points here...

The 0.7 docs are probably just wrong. They also are missing the 1.39 feature flag from that table.
Since we are upping the MSRV and releasing a new semver-incompatible version, we will probably want to change the default feature-flagged version to be whatever is stable when we release 0.8 (likely 1.70 /1.71 which will effectively mean 1.61 as there are no other flags since then).

I'm generally planning on releasing one more 0.7 so I can fix those docs (and maybe cause some merge conflicts 😬).

Enet4 commented 1 year ago

Would you mind pulling Update compile_error test snapshots out into its own PR? It's not really about updating to 1.56, just that the current stable output has changed. (also I want it for #383 😇)

Makes sense indeed. I'll get to that once I'm able.

Maybe also rustfmt src/lib.rs if that's more drift from stable.

That was caused by the removal of the non-exhaustive lock field _other. I might just fuse the commit with a fixup.

Hmm, I thought it was because Rust 1.34 didn't know the async keyword (only 1.39+). That file also has a switch on 1.61, but that shouldn't be relevant for merging the file.

I guess it's easy to try if the code builds on Rust 1.56 anyway, and make changes if so. I'll look into it.

Hmm, there's two points here...

The 0.7 docs are probably just wrong. They also are missing the 1.39 feature flag from that table.

Since we are upping the MSRV and releasing a new semver-incompatible version, we will probably want to change the default feature-flagged version to be whatever is stable when we release 0.8 (likely 1.70 /1.71 which will effectively mean 1.61 as there are no other flags since then).

OK, in that case I will not touch the documentation (which already claims that rust_v1_61 would be enabled).

I'm generally planning on releasing one more 0.7 so I can fix those docs (and maybe cause some merge conflicts 😬).

*git rebase intensifies*

Stargateur commented 1 year ago

You can also add:

[package]
rust-version = "1.56"

to all Cargo.toml file

shepmaster commented 1 year ago

Looking good. I made one small tweak to wording and force pushed (also rebased for good measure).

shepmaster / snafu