Skip to content

clippy!: Box large enum variants #277

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

Merged
merged 1 commit into from
Jul 11, 2025

Conversation

ValuedMammal
Copy link
Collaborator

@ValuedMammal ValuedMammal commented Jul 9, 2025

Description

This is a fix to address clippy lints

  • clippy::large_enum_variant
  • clippy::result_large_error

BREAKING: The types LoadMismatch, and CreateWithPersistError are changed as a result of the now boxed variants.

fixes #245

Checklists

All Submissions:

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@ValuedMammal ValuedMammal added this to the Wallet 3.0.0 milestone Jul 9, 2025
@ValuedMammal ValuedMammal self-assigned this Jul 9, 2025
@ValuedMammal ValuedMammal added the api A breaking API change label Jul 9, 2025
@ValuedMammal ValuedMammal moved this to Needs Review in BDK Wallet Jul 9, 2025
@ValuedMammal ValuedMammal moved this from Needs Review to In Progress in BDK Wallet Jul 9, 2025
@coveralls
Copy link

coveralls commented Jul 9, 2025

Pull Request Test Coverage Report for Build 16200528356

Details

  • 4 of 13 (30.77%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.04%) to 84.69%

Changes Missing Coverage Covered Lines Changed/Added Lines %
wallet/src/wallet/mod.rs 4 7 57.14%
wallet/src/wallet/persisted.rs 0 6 0.0%
Files with Coverage Reduction New Missed Lines %
wallet/src/wallet/mod.rs 1 81.03%
Totals Coverage Status
Change from base Build 16052072493: -0.04%
Covered Lines: 6577
Relevant Lines: 7766

💛 - Coveralls

This is a fix to address the clippy lints

- `clippy::large_enum_variant`
- `clippy::result_large_error`

BREAKING: The types `LoadMismatch` and `CreateWithPersistError`
are changed as a result of the now boxed variants.
@ValuedMammal ValuedMammal moved this from In Progress to Needs Review in BDK Wallet Jul 10, 2025
Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 650c3ee

@evanlinjin evanlinjin merged commit 3fabe09 into bitcoindevkit:master Jul 11, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Wallet Jul 11, 2025
@ValuedMammal ValuedMammal deleted the fix/clippy-lints branch July 14, 2025 17:28
notmandatory added a commit that referenced this pull request Jul 19, 2025
14d6a62 Revert "clippy!: `Box` large enum variants" (Steve Myers)

Pull request description:

  Reverts #277

  Per discussion on team chat yesterday we need to revert this since it's a breaking change and wasn't meant to be merged until the 3.0 milestone. ValuedMammal will have to re-submit a replacement PR.

ACKs for top commit:
  ValuedMammal:
    ACK 14d6a62

Tree-SHA512: 110446e6e7af41f8303d540755202b5df2c67340fd3f608756acb8335a378cf76e1e3a1c79f06c6170ed1ac0ca25413d8575c2be034882632d86a015297be285
@ValuedMammal ValuedMammal restored the fix/clippy-lints branch July 28, 2025 17:40
@ValuedMammal ValuedMammal deleted the fix/clippy-lints branch August 3, 2025 20:15
evanlinjin added a commit to bitcoindevkit/bdk that referenced this pull request Aug 7, 2025
418753f refactor!: `Box` changeset in `StoreErrorWithDump` (codingp110)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description
  Fixes #1991. As the linked issue mentions `Box`ing helps reduce memory overhead and [limits the size of enums with `StoreErrorWithDump` as variant](bitcoindevkit/bdk_wallet#277 (comment)).

  Breaking: The enum `StoreErrorWithDump` and the functions `load`,`dump` and `load_or_create` have been changed.

  ### Changelog Notice
  ```
     Changed
     - `changeset` field of `StoreErrorWithDump` is now of type `Option<Box<C>>`
  ```

  ### Checklists

  #### All Submissions:

  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)

ACKs for top commit:
  evanlinjin:
    ACK 418753f

Tree-SHA512: c09d2b14989f62613283b728cd8f0eec849a2f3980b07e4289d55289a2834b27c32ff0fcb89706c636441c02b385fbb4a4175e93615f6e16fa2e2e0cfb0e4e26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api A breaking API change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

ci: fix large enum and error types to clippy recommended sizes
3 participants