Skip to content

Remove Default derive from AccountMapEntry#8146

Merged
kskalski merged 1 commit intoanza-xyz:masterfrom
kskalski:ks/accts_map_entry_no_default
Sep 23, 2025
Merged

Remove Default derive from AccountMapEntry#8146
kskalski merged 1 commit intoanza-xyz:masterfrom
kskalski:ks/accts_map_entry_no_default

Conversation

@kskalski
Copy link
Copy Markdown

@kskalski kskalski commented Sep 23, 2025

Problem

Default account map entry isn't really a valid value that production code creates to put into the map, there are just a few uses in tests that create artificial state.

Summary of Changes

  • remove Default derive from AccountMapEntry
  • add empty_for_tests guarded with cfg(test) such that tests can still use a convenience function

{
// add an entry with an empty slot list
let val = Arc::new(AccountMapEntry::<u64>::default());
let val = Arc::new(AccountMapEntry::<u64>::empty_for_tests());
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We could possibly create entry with new(defaults...), but I think having function encapsulates this use case better and hides what exact values are used for initialization

@kskalski kskalski marked this pull request as ready for review September 23, 2025 02:27
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.0%. Comparing base (033b4a2) to head (79ed653).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8146   +/-   ##
=======================================
  Coverage    83.0%    83.0%           
=======================================
  Files         826      826           
  Lines      362192   362199    +7     
=======================================
+ Hits       300755   300761    +6     
- Misses      61437    61438    +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

I agree this shouldn't drive Default.

@kskalski kskalski merged commit d0d5666 into anza-xyz:master Sep 23, 2025
43 checks passed
@kskalski kskalski deleted the ks/accts_map_entry_no_default branch September 23, 2025 04:06
@anza-xyz anza-xyz deleted a comment from Likvid2022 Sep 26, 2025
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.

3 participants