search

bitcoindevkit / rust-esplora-client

Bitcoin Esplora API client library. Supports plaintext, TLS and Onion servers. Blocking or async.
MIT License
28 stars 44 forks source link

Remove unused dev-dependencies #52

Closed tcharding closed 1 year ago

tcharding commented 1 year ago

The two dependencies zip and base64ct are not used, remove them.

If this was meant as a way to fix the version of these libs we can always pin in CI.

vladimirfomene commented 1 year ago

@tcharding yes we pinned those dependencies to fix MSRV of some of our dependencies so that they come down to 1.57.0 which is BDK's current MSRV. How will pinning in CI work? Will pinning in CI ensure that rust-esplora-client maintain MSRV of 1.57.0 during release?

notmandatory commented 1 year ago

Both zip and base64ct are needed to run the tests with 1.57 so we need them in [dev-dependencies].

They're not included when making non-dev builds

$ cargo tree -i base64ct -e=no-dev            
warning: nothing to print.
...
$ cargo tree -i zip -e=no-dev
warning: nothing to print.
...
notmandatory commented 1 year ago

It would be nice to get these versions pinned in electrsd, issue and discussion here: https://github.com/RCasatta/electrsd/issues/56

tcharding commented 1 year ago

In rust-bitcoin we run a shell script in CI for test. And in that we pin dependencies if we are running the script while using the MSRV toolchain

You can check out https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/contrib/test.sh if interested, and here is the relevant snippet:

# Pin dependencies as required if we are using MSRV toolchain.
if cargo --version | grep "1\.48"; then
    # 1.0.157 uses syn 2.0 which requires edition 2018
    cargo update -p serde --precise 1.0.156
fi
tcharding commented 1 year ago

I'm going to close this PR because I'm unlikely to get to changing your CI, and I don't really want to mess with the CI of another project, seems a bit rude :)

vladimirfomene commented 1 year ago

In rust-bitcoin we run a shell script in CI for test. And in that we pin dependencies if we are running the script while using the MSRV toolchain

You can check out https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/contrib/test.sh if interested, and here is the relevant snippet:

# Pin dependencies as required if we are using MSRV toolchain.
if cargo --version | grep "1\.48"; then
    # 1.0.157 uses syn 2.0 which requires edition 2018
    cargo update -p serde --precise 1.0.156
fi

Thanks for sharing this! Will discuss with the team if it is something that makes sense for us.