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

move msrv from clippy config to Cargo.toml #371

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

danieleades
Copy link
Contributor

@danieleades danieleades commented Jan 17, 2023

clippy now supports parsing the 'rust version' from cargo.toml

setting it in Cargo.toml has other benefits, such as allowing compilation to exit gracefully if the compiler version doesn't meet the rust-version minimum

this patterns sets the same value for all members of the workspace.

@dwrensha
Copy link
Member

rust-version got added to capnp/Cargo.toml in #356
If we add it to the workspace Cargo.toml, should we keep or remove that one?
If it's in the workspace Cargo.toml, does it get picked up by packaged crates?

@danieleades
Copy link
Contributor Author

rust-version got added to capnp/Cargo.toml in #356 If we add it to the workspace Cargo.toml, should we keep or remove that one? If it's in the workspace Cargo.toml, does it get picked up by packaged crates?

ah good catch. So in this case the version set in the root is inherited by all the members of the workspace, but they can override it locally.
so the field should be removed from capnp/Cargo.toml in favour of the root.

i'll update

@danieleades danieleades marked this pull request as draft January 17, 2023 14:23
@danieleades danieleades marked this pull request as ready for review January 17, 2023 15:07
@dwrensha
Copy link
Member

It looks like maybe the workspace-level msrv does not get picked up by packaging?

$ cargo package -p capnp
   Packaging capnp v0.16.0 (/home/dwrensha/src/capnproto-rust/capnp)
   Verifying capnp v0.16.0 (/home/dwrensha/src/capnproto-rust/capnp)
    Updating crates.io index
   Compiling capnp v0.16.0 (/home/dwrensha/src/capnproto-rust/target/package/capnp-0.16.0)
    Finished dev [unoptimized + debuginfo] target(s) in 1.19s
    Packaged 40 files, 418.9KiB (70.7KiB compressed)
dwrensha@hafnium:~/src/capnproto-rust$ cat target/package/capnp-0.16.0/Cargo.toml | grep rust-version
[not found]

I suppose that means we need it in both places?

@danieleades
Copy link
Contributor Author

Looks like I misunderstood how the inheritance works. The docs here make it more clear - https://doc.rust-lang.org/cargo/reference/workspaces.html#the-package-table

you need to opt in per-key inside the member package

@dwrensha dwrensha merged commit c1b4c3a into capnproto:master Jan 19, 2023
@dwrensha
Copy link
Member

Thanks!

@danieleades danieleades deleted the msrv branch January 20, 2023 07:13
ErikMcClure pushed a commit to Fundament-Software/capstone-rs that referenced this pull request Mar 25, 2024
move msrv from clippy config to Cargo.toml
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