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

Simplify BlockCommitmentCache slot info #11106

Merged
merged 2 commits into from
Jul 17, 2020

Conversation

CriesofCarrots
Copy link
Contributor

Problem

BlockCommitmentCache and CacheSlotInfo store the same data, but in different ways. No functional reason for this, and it just obfuscates the code.

Summary of Changes

Reorganization only: Store a CacheSlotInfo in BlockCommitmentCache.

Closes #11104

@CriesofCarrots CriesofCarrots requested a review from garious July 17, 2020 04:11
Copy link
Contributor

@garious garious left a comment

Choose a reason for hiding this comment

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

Much clearer. Thanks for doing this!

@codecov
Copy link

codecov bot commented Jul 17, 2020

Codecov Report

Merging #11106 into master will increase coverage by 0.0%.
The diff coverage is 89.0%.

@@           Coverage Diff           @@
##           master   #11106   +/-   ##
=======================================
  Coverage    82.1%    82.1%           
=======================================
  Files         319      319           
  Lines       73367    73386   +19     
=======================================
+ Hits        60287    60309   +22     
+ Misses      13080    13077    -3     

@garious
Copy link
Contributor

garious commented Jul 17, 2020

CacheSlotInfo seems like a strange name. I don't get what that's supposed to mean. CommitmentSlots perhaps?

@CriesofCarrots CriesofCarrots merged commit fdff681 into solana-labs:master Jul 17, 2020
@CriesofCarrots
Copy link
Contributor Author

CacheSlotInfo seems like a strange name. I don't get what that's supposed to mean. CommitmentSlots perhaps?

Perfect timing ;) CommitmentSlots is definitely better. I'll PR a rename

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