-
Notifications
You must be signed in to change notification settings - Fork 268
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(ui): Introduce the AllRemoteEvents
type
#4370
refactor(ui): Introduce the AllRemoteEvents
type
#4370
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4370 +/- ##
==========================================
- Coverage 85.18% 85.15% -0.03%
==========================================
Files 280 280
Lines 30761 30773 +12
==========================================
+ Hits 26203 26206 +3
- Misses 4558 4567 +9 ☔ View full report in Codecov by Sentry. |
AllRemoteEvents
type
904548d
to
84bd2e1
Compare
This patch replaces `VecDeque<EventMeta>` by `AllRemoteEvents` which is a wrapper type around `VecDeque<EventMeta>`, but this new type aims at adding semantics API rather than a generic API. It also helps to isolate the use of these values and to know precisely when and how they are used. As a first step, `AllRemoteEvents` implements a generic API to not break the existing code. Next patches will revisit that a little bit step by step.
This patch renames `AllRemoteEvents::back` to `last` so that it now gets a specific semantics instead of being generic.
Having a mutable iterator can be dangerous and is probably too generic regarding the safety we want to add around the `AllRemoteEvents` type. This patch removes `iter_mut` and replaces it by its only use case: `get_by_event_id_mut`.
84bd2e1
to
5689a6f
Compare
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.
lgtm 👍
A shepherdess gathering a herd of sheep.
Build on top of #4369.
The first patch replaces
VecDeque<EventMeta>
byAllRemoteEvents
whichis a wrapper type around
VecDeque<EventMeta>
, but this new type aimsat adding semantics API rather than a generic API. It also helps to
isolate the use of these values and to know precisely when and how they
are used.
As a first step,
AllRemoteEvents
implements a generic API to not breakthe existing code. Next patches are revisiting that a little bit step
by step:
AllRemoteEvents::back
becomeslast
to add semantics.AllRemoteEvents::iter_mut
are removed andget_by_event_id_mut
is created.
This PR should be reviewed patch-by-patch.
EventCache
storage #3280