Skip to content

Boxes inner SnapshotError in BankForksUtilsError::BankFromSnapshotsArchive#6625

Merged
brooksprumo merged 1 commit intoanza-xyz:masterfrom
brooksprumo:clippy/large-err/bank-forks-utils
Jul 1, 2025
Merged

Boxes inner SnapshotError in BankForksUtilsError::BankFromSnapshotsArchive#6625
brooksprumo merged 1 commit intoanza-xyz:masterfrom
brooksprumo:clippy/large-err/bank-forks-utils

Conversation

@brooksprumo
Copy link
Copy Markdown

Problem

Issue #6278: The BankForksUtilsError triggers the result_large_err clippy lint. This is because a variant, BankFromSnapshotsArchive, is too big. To fix this, and since this is only needed in the error case, we can box the inner SnapshotError (which is 80 bytes).

Similar to #6293.

Summary of Changes

Boxes inner SnapshotError.

@brooksprumo brooksprumo self-assigned this Jun 17, 2025
@brooksprumo brooksprumo linked an issue Jun 17, 2025 that may be closed by this pull request
@brooksprumo brooksprumo marked this pull request as ready for review June 17, 2025 21:31
@brooksprumo brooksprumo requested a review from steviez June 17, 2025 21:32
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 17, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.3%. Comparing base (af62b25) to head (6e16480).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #6625     +/-   ##
=========================================
- Coverage    83.3%    83.3%   -0.1%     
=========================================
  Files         853      853             
  Lines      378132   378132             
=========================================
- Hits       315290   315259     -31     
- Misses      62842    62873     +31     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bxczhu
Copy link
Copy Markdown

bxczhu commented Jun 24, 2025

Hi team, can we merge this and unblock bumping rust to 1.87.0? Thanks!

Copy link
Copy Markdown

@steviez steviez left a comment

Choose a reason for hiding this comment

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

Just the one question. Also, would you mind rekicking CI on this ? It has been sitting for a couple weeks (my fault, not yours) so just wanna make sure things haven't changed "under the hood".

Also, can you re-kick your other ones as well; I'll try to review those in quick succession after I see rebases

Comment thread ledger/src/bank_forks_utils.rs
@brooksprumo brooksprumo force-pushed the clippy/large-err/bank-forks-utils branch from f24385e to 6e16480 Compare July 1, 2025 18:54
@brooksprumo
Copy link
Copy Markdown
Author

Also, would you mind rekicking CI on this ? It has been sitting for a couple weeks (my fault, not yours) so just wanna make sure things haven't changed "under the hood".

Done!

@brooksprumo brooksprumo merged commit 21f770a into anza-xyz:master Jul 1, 2025
41 checks passed
@brooksprumo brooksprumo deleted the clippy/large-err/bank-forks-utils branch July 1, 2025 19:45
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.

New result_large_err clippy lint blocks upgrading Rust to 1.87.0

4 participants