-
Notifications
You must be signed in to change notification settings - Fork 678
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
[store] Introduce Epoch store adapter #12856
Conversation
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.
nice, thanks!
} | ||
|
||
// Iterate over all the epoch infos in store | ||
pub fn iter_epoch_info<'a>(&'a self) -> Box<dyn Iterator<Item = (EpochId, EpochInfo)> + 'a> { |
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.
I think we can avoid using Box
+ dyn
here and just return impl Iterator
, but feel free to merge as it is for now, I will try that and submit a separate PR if it works
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.
Nice, you're right! I can do away with the Box and just use impl Iterator. I was originally going off what we had in DBIterator<'a>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12856 +/- ##
=======================================
Coverage 70.39% 70.39%
=======================================
Files 851 853 +2
Lines 174188 174114 -74
Branches 174188 174114 -74
=======================================
- Hits 122614 122570 -44
+ Misses 46327 46321 -6
+ Partials 5247 5223 -24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Follow up on all the store adapter work.
In future I'll add update adapter as well