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

Remove unused dev-dependencies #52

Closed

Conversation

tcharding
Copy link
Contributor

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.

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
Copy link
Contributor

@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
Copy link
Member

notmandatory commented Jun 14, 2023

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
Copy link
Member

It would be nice to get these versions pinned in electrsd, issue and discussion here: RCasatta/electrsd#56

@tcharding
Copy link
Contributor Author

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
Copy link
Contributor Author

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 :)

@tcharding tcharding closed this Jun 18, 2023
@vladimirfomene
Copy link
Contributor

vladimirfomene commented Jun 19, 2023

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.

@notmandatory notmandatory added this to the Release 0.6.0 milestone Jul 3, 2023
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.

None yet

3 participants