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

bump stable rustc version to 1.65 #140

Merged
merged 2 commits into from
Feb 14, 2023

Conversation

rajarshimaitra
Copy link
Contributor

@rajarshimaitra rajarshimaitra commented Jan 18, 2023

Description

As BDK bumped its stable to 1.65, it is safe for us to move it up also.

Notes to the reviewers

1.65 cause clippy warning that Eq isn't derived while PartialEq is derived for the commands. We can't derive Eq for commands, as there is a fee_rate which is f32 which isn't Eq, and this effect cascades to all structures. We also need PartialEq for the tests.

A few options were:

  • allow the clippy warning conditionally for 1.65, 1.57 doesn't have that lint. The way to do that is by creating custom features for different versions and making a build script that activates specific features for specific versions. Feature gate the clippy lint. or use https://github.com/dtolnay/rustversion which is an extra dep.

  • Remove PartialEq and create a method on CliOpts to translate it into Vec<String>, and then perform the tests.

  • Make fee_rate u32 and derive Eq for all.

I went for the last option as it seemed less complex, and not too bad(?). For bdk_cli as majorly a testing tool, it might not need floating point granularity and users can still test out with u32 fee_rates. Internally it converts it back to f32.

Suggestions welcome.

Changelog notice

  • Bumped rustc stable to 1.65.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@rajarshimaitra rajarshimaitra marked this pull request as draft January 18, 2023 04:49
@rajarshimaitra rajarshimaitra force-pushed the bump-stable-to-1.65 branch 2 times, most recently from d020f7c to 47a3ea1 Compare January 18, 2023 05:55
@rajarshimaitra rajarshimaitra marked this pull request as ready for review January 18, 2023 06:52
@notmandatory notmandatory added this to the Release 0.27.0 milestone Feb 14, 2023
@notmandatory
Copy link
Member

I removed the derive for Eq on a CliOpts and a few others so that we can leave the FeeRate amount as a f32. Clippy for 1.65 is still happy with all the changes.

@notmandatory notmandatory merged commit c25dc0c into bitcoindevkit:master Feb 14, 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.

2 participants