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

Document MSRV as 1.76 #93

Merged
merged 2 commits into from
Jan 25, 2025
Merged

Document MSRV as 1.76 #93

merged 2 commits into from
Jan 25, 2025

Conversation

goffrie
Copy link
Contributor

@goffrie goffrie commented Jan 23, 2025

#83 uses inline const which isn't stable until 1.79

@jplatte
Copy link
Contributor

jplatte commented Jan 23, 2025

I think it would be good to add it to Cargo.toml as well.

@jneem
Copy link
Owner

jneem commented Jan 24, 2025

Huh, I don't have any real objection to bumping MSRV, but I'm concerned that we didn't catch this in CI. The CI runs all the tests on 1.51 -- do you know why it didn't catch this?

@goffrie
Copy link
Contributor Author

goffrie commented Jan 24, 2025

I don't think CI is actually running the requested toolchain version, I'm pretty sure it's always running stable.

I tried running cargo test on 1.51 and it already failed trying to build the latest libc (since this repo doesn't have Cargo.lock checked in).

@jplatte
Copy link
Contributor

jplatte commented Jan 24, 2025

Looks like you're using the deprecated actions-rs/toolchain@v1. IIRC you have to use its override or default option to actually have the installation affect what cargo commands without +toolchain do. Most Rust GitHub CI workflows have switched to https://github.com/dtolnay/rust-toolchain.

@goffrie
Copy link
Contributor Author

goffrie commented Jan 24, 2025

#94 adds override: true. switching seems like a reasonable idea too.

@goffrie goffrie marked this pull request as draft January 24, 2025 22:05
@goffrie
Copy link
Contributor Author

goffrie commented Jan 24, 2025

#95 should let us lower the msrv to 1.65 at least

Copy link
Owner

@jneem jneem left a comment

Choose a reason for hiding this comment

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

After the various other changes, 1.76 is the minimal version (with triomphe as the bottleneck).

Cargo.toml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@goffrie goffrie marked this pull request as ready for review January 25, 2025 02:41
@goffrie goffrie changed the title Document MSRV as 1.79 Document MSRV as 1.76 Jan 25, 2025
@jneem jneem merged commit 23522c7 into jneem:main Jan 25, 2025
15 checks passed
@goffrie goffrie deleted the patch-1 branch January 28, 2025 03:41
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.

3 participants