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

chore: Use cargo-no-dev-deps to check msrv #1570

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

tottoto
Copy link
Collaborator

@tottoto tottoto commented Nov 15, 2023

Motivation

Removes the effects from dev-dependencies to MSRV, and makes easier to specify public crates for checking.

Solution

Uses cargo-no-dev-deps to check msrv.

@tottoto tottoto force-pushed the msrv-checking branch 2 times, most recently from 7204378 to 8f22932 Compare May 21, 2024 12:16
@djc
Copy link
Contributor

djc commented May 21, 2024

How is this different from just using cargo check --lib?

@tottoto
Copy link
Collaborator Author

tottoto commented May 21, 2024

It can remove the effect to Cargo.lock from crates in dev-dependencies.

@djc
Copy link
Contributor

djc commented May 21, 2024

It can remove the effect to Cargo.lock from crates in dev-dependencies.

Can you elaborate a little bit? Why is that important (how do we rely upon it here)? I've implemented this using cargo check --lib in a bunch of other repos and have not found that many issues with it -- and I'd prefer the simplicity of that approach over bringing in this external tool (that I've not seen used before).

@tottoto
Copy link
Collaborator Author

tottoto commented May 21, 2024

It is important to test in an environment as close as possible to the environment in which it will be actually released.

Let's consider the following example.

A project depends on crate-a 0.1 as dependencies and crate-b as dev-dependencies, and crate-b depends on crate-a =0.1.0 as dependencies. When crate-a has a version 0.1.1, checking (with the affected Cargo.lock by dev-dependencies) is targeted to crate-a 0.1.0, not 0.1.1 which is users actually faced. And more thinking the case if crate-a bumped the MSRV at 0.1.1 from 0.1.0, the MSRV checking is broken.

Although such an example may seem trivial, one of the important roles of testing is to prepare for such risks.

@djc
Copy link
Contributor

djc commented May 21, 2024

@LucioFranco any thoughts on the trade-off here? It's nice to avoid dev-deps for MSRV checks but it feels to me like the extra complexity of depending on an external tool is probably not worth it.

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

@djc to me it seems fine to depend on a tool by the author of that one. We can always revert this back pretty easily later. I think this makes a lot of sense.

@LucioFranco LucioFranco added this pull request to the merge queue Jun 10, 2024
Merged via the queue into hyperium:master with commit dfa0893 Jun 10, 2024
16 checks passed
@tottoto tottoto deleted the msrv-checking branch June 11, 2024 12:49
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