search

rust-lang / log

Logging implementation for Rust
https://docs.rs/log
Apache License 2.0
2.16k stars 250 forks source link

Remove rustversion dev-dependency #568

Closed Thomasdezeeuw closed 1 year ago

Thomasdezeeuw commented 1 year ago

Since the MSRV is now 1.60.

Thomasdezeeuw commented 1 year ago

The minimal versions issue should be resolved by updating to proc-macro2 v1.0.60 (specifically in https://github.com/dtolnay/proc-macro2/commit/e31d61910049e097afdd3d27c37786309082bdcb). But I think this has to happen in serde_derive?

Thomasdezeeuw commented 1 year ago

The minimal versions issue should be resolved by updating to proc-macro2 v1.0.60 (specifically in dtolnay/proc-macro2@e31d619). But I think this has to happen in serde_derive?

Oh both serde_derive and sval_derive define proc-macro2 = "1.0" (https://github.com/serde-rs/serde/blob/015e39776fcd951d208d7fbb5f4a4de4efb4445f/serde_derive/Cargo.toml#L25 and https://github.com/sval-rs/sval/blob/3cbda0f19ac37260b44f16233944529f04666e05/derive/Cargo.toml#L25), so this is going to be a bit annoying.

KodrAus commented 1 year ago

Ah at first I was reading minimal versions as MSRV and wasn’t following what you meant. Now I get it.

I’m happy to add minimal versions to CI in sval and bump proc-macro2, but it might be easier for us to simply remove those derive dependencies in log. It might get a little tedious, depending on where we use them though.

KodrAus commented 1 year ago

Alternatively we could add proc-macro2 as a dev dependency with a comment on why it’s there. That might be the least painful.

Thomasdezeeuw commented 1 year ago

Alternatively we could add proc-macro2 as a dev dependency with a comment on why it’s there. That might be the least painful.

I've went with this option as it's the easiest, even though it's a little hacky.

KodrAus commented 1 year ago

I’ll run through later and see if I can yank the derive crates out as dependencies altogether.