search

rust-embedded / svd2rust

Generate Rust register maps (`struct`s) from SVD files
Apache License 2.0
697 stars 150 forks source link

more bagdes, bump deps #741

Closed burrbull closed 1 year ago

burrbull commented 1 year ago

cc @rust-embedded/tools

thejpster commented 1 year ago

In the absence of a Cargo.lock file, the package bumps won't make any difference because the resolver will pick the highest one anyway. Technically we should specify the lowest we can get away with, but usually it's whatever was the latest at the point the dep was added.

So I would either add the Cargo.lock file (and I'll want a second opinion from @rust-embedded/tools on that), or I wouldn't bump the dependencies and I wouldn't show the dependency check badge.

I lean towards adding the lock file myself.

adamgreig commented 1 year ago

Bumping the Cargo.toml will still ensure everyone has at least that version, but yea since this is typically installed as a binary with cargo install you'd expect everyone to get the latest version anyway.

Unless someone runs cargo install --locked svd2rust I don't think Cargo will pay attention to the lockfile if we did add one, so I'm not sure there's much point adding it. It might make some difference to people working on it from a Git checkout, but I think that's just developers. I'd expect mostly it's best to use the latest available version of dependencies anyway, and the CI will be checking that too.

Still it shouldn't hurt to bump the deps here to whatever the latest version we want to use is. I wouldn't bother with the dependency check badge myself but I don't mind having it. I think @burrbull wants to do a release ASAP, so I think let's merge this now and we can revisit a lockfile after the release.

adamgreig commented 1 year ago

Just adding on to the last comment, Cargo does recommend binaries like svd2rust publish a lockfile so someone could use --locked if they wanted to, so perhaps we should be including it here too, even though it won't be used by default.

thejpster commented 1 year ago

Yeah I think we should. Further I think we should encourage the use of --locked in the docs/README and also ship binaries to use with cargo binstall which were built with locked dependencies.

Emilgardis commented 1 year ago

I'm happily surprised about the inclusion of a Cargo.lock, we've had discussions about this before and ended up not doing it.

cc https://github.com/rust-embedded/svd2rust/issues/399