search

summa-tx / coins

Rust implementations of BIP32/39 and Ledger device comms
Other
90 stars 31 forks source link

ci: commit Cargo.lock #117

Closed agostbiro closed 1 year ago

agostbiro commented 1 year ago

Fixes #116

I could reproduce the issue when running cargo build --verbose --target wasm32-unknown-unknown --no-default-features --features "browser" --locked in the repo root without a Cargo.lock file.

It should be fixed by committing the Cargo.lock file which I did in this PR. This should have no undesirable effects as it'll be ignored when users of the library compile it.

prestwich commented 1 year ago

hmmm, I think i'd be more inclined to remove the --locked argument. Official docs recommend not commiting the lockfile for libs

https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html

agostbiro commented 1 year ago

Sure, that would work too.

I think the docs are more of a guidance as committing the Cargo.lock file has no effect from the end user's perspective for libraries. But it's ok to commit it if reproducible builds in the CI are important (eg. ethers-rs and elliptic-curves do it). The advantage of not having it committed imo is that the CI can serve as a canary if one of the dependencies has a new version that is allowed by semantic versioning, but causes some issues.

Wdyt, want me to remove the --locked argument instead?

prestwich commented 1 year ago

I went ahead and removed the locked arg this morning, thank you!