-
Notifications
You must be signed in to change notification settings - Fork 970
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(share/availability): Move window and constants to share/availability pkg #3906
refactor(share/availability): Move window and constants to share/availability pkg #3906
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3906 +/- ##
==========================================
+ Coverage 44.83% 45.27% +0.43%
==========================================
Files 265 308 +43
Lines 14620 21853 +7233
==========================================
+ Hits 6555 9894 +3339
- Misses 7313 10884 +3571
- Partials 752 1075 +323 ☔ View full report in Codecov by Sentry. |
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.
Handling pruning windows just got a whole lot easier! It would be awesome to rename the variables so it's clear which window they're using. Right now, names like "window," "samplingWindow," and "availWindow" can be confusing inside the component. Being more specific about which of the two new windows is being used would help a lot.
Don’t forget the functions too, like changing WithAvailabilityWindow to WithStorageWindow. These tweaks would make the code easier to understand for everyone.
I plan to move those parameters outside of those components entirely (with exception of core pkg n which case I will name it to |
35a8aa7
to
06179a2
Compare
fa7ba77
to
e8b6cb0
Compare
Should #3912 be closed? |
no #3912 is based on this PR @cristaloleg |
…lder from share package
Partially addresses #3394
availability.Window
to the nodebuilder only, usetime.Duration
everywhere elsetime.Duration(0)
-- will be removed entirely in follow-up PRs.