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

Set MSRV to 1.64.0 in package metadata #1343

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nickelc
Copy link
Contributor

@nickelc nickelc commented Mar 11, 2023

clippy respects the rust version set in the package metadata and wouldn't break the build job for newer rust features.

The current clap version in use dictates 1.64.0.

clippy respects the rust version set in the package metadata and
wouldn't break the build job for newer rust features.
@nickelc nickelc marked this pull request as draft March 11, 2023 10:16
Make sure the line starts with `version` and doesn't pick up `rust-version`.
@nickelc nickelc marked this pull request as ready for review March 11, 2023 10:54
@dandavison
Copy link
Owner

Hi @nickelc I trust your changes but would you mind spelling out for me exactly what problem this change is solving?

@nickelc
Copy link
Contributor Author

nickelc commented Mar 11, 2023

First it documents the MSRV for readers and then it limits clippy to only issue warnings for lints that are compatible with the MSRV.

This reduces the possibility of a broken clippy build job and allows the version requirements to be changed deliberately.

For example, the #[derive(Default)] on enum (#1341) was stabilized with 1.62 and I just saw in another project that satisfying clippy bumped the project's MSRV from 1.56 to 1.62.

delta could also be checked against this MSRV with a CI job.

@nickelc
Copy link
Contributor Author

nickelc commented Mar 11, 2023

delta could also be checked against this MSRV with a CI job.

This is how the CI job could look like.

msrv:
name: MSRV
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v3
- name: Get MSRV from package metadata
id: msrv
run: grep rust-version Cargo.toml | cut -d'"' -f2 | sed 's/^/version=/' >> $GITHUB_OUTPUT
- name: Install ${{ steps.msrv.outputs.version }} toolchain
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ steps.msrv.outputs.version }}
- run: cargo check

@dandavison
Copy link
Owner

How will I know when to update the MSRV?

@nickelc
Copy link
Contributor Author

nickelc commented Apr 26, 2023

How will I know when to update the MSRV?

A build job would allow to check PRs against the MSRV #1343 (comment) and then decide to deliberately bump the MSRV.

@dandavison
Copy link
Owner

It might well be my lack of familiarity with standard Rust practices but my worry is that this is more moving parts to maintain. When I'm in doubt I sometimes look at how bat and ripgrep do things, as examples of Rust CLI projects, so I'll try to do that.

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.

2 participants