-
Notifications
You must be signed in to change notification settings - Fork 659
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
refactor: restrict ChainUpdate fields visibility #10388
Conversation
16aa155
to
66682e0
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #10388 +/- ##
==========================================
+ Coverage 71.99% 72.02% +0.02%
==========================================
Files 718 718
Lines 144524 144520 -4
Branches 144524 144520 -4
==========================================
+ Hits 104050 104089 +39
+ Misses 35722 35686 -36
+ Partials 4752 4745 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
let mut store_update = chain_update.chain_store_update.store().store_update(); | ||
let epoch_manager = self.epoch_manager.clone(); | ||
let mut chain_store_update = self.store.store_update(); | ||
let mut store_update = store.store_update(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aah this is super confusing. store
can be ChainStore or Store depending on whether it comes from self.store or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future we might want to rename all references to store
of type ChainStore to chain_store
maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I agree, chain_store
would be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up from PR #10388
Removes
pub(crate)
fromChainUpdate
fields. All usage is refactored to directly useChain
's fields.This is a follow-up for #10380.