-
Notifications
You must be signed in to change notification settings - Fork 265
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(event cache): a few AllEventsCache
refactorings
#4471
Conversation
…pend_related_event`
…lEventsCache` No code changes.
Reduce indent, simplify parameter type, etc.
…wkward The last parameter was passed to handle recursivity: introduce a small `_rec` helper function, and call that instead, so external callers don't have to interact with the weird API.
… of `AllEventsCache`
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.
Thanks, the changes make the code way more readable!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4471 +/- ##
==========================================
- Coverage 85.07% 85.07% -0.01%
==========================================
Files 283 283
Lines 31775 31777 +2
==========================================
+ Hits 27033 27034 +1
- Misses 4742 4743 +1 ☔ View full report in Codecov by Sentry. |
I was investigating a potential deadlock with the event cache storage, and only found a few places where to make the code a bit more idiomatic and more readable.
Should be reviewed commit by commit.