ffi: Refactor timeline creation and lifetimes #5058
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stefanceriu
requested review from
andybalaam,
Hywan and
bnjbvr
and removed request for
a team and
andybalaam
bnjbvr
reviewed
May 19, 2025
bnjbvr
requested changes
May 19, 2025
|
(removing Ivan from the reviewer list, as I've reviewed it.) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5058 +/- ##
=======================================
Coverage 85.85% 85.86%
=======================================
Files 325 325
Lines 35937 35920 -17
=======================================
- Hits 30855 30841 -14
+ Misses 5082 5079 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bnjbvr
changed the title
bnjbvr
approved these changes
May 21, 2025
There was a problem hiding this comment.
lgtm, thanks! we can likely get rid of more room wrappers in the SDK, after this…
Ideally, in terms of clean history management, we'd do some tidying up of the commits here:
- the "make ci happy" only contains changes that ought to be included in other commits.
git absorbis your friend in this kind of situations! - the commit introducing the
OnceLockfor the UTD hook manager ought to be squashed with the other one that introduced theArc<Mutex<...>>because this commit is undoing things that another commit made (fixup commit FTW!).
…service and room list items
… timeline As per the plan defined in #4718: ``` the room_list_service::room::RoomInner shouldn't make use of its inner timeline; it's only used in a direct getter, or to compute the latest room event, but it's not working as intended, since local echoes aren't properly displayed in the room list. This non-working feature can be removed, in favor of #4112 ```
…callback (dropped in favor of the `ClientSessionDelegate`)
… the hook manager isn't configured
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the plan laid out in #4718: it removes long living timeline instances from the room list service and its items in favor of directly creating a timeline through its parent room and the
timeline_with_configurationmethod.In order to do so it also moves the previously internally configured UTD hook to the FFI client and exposes an option to use it on the aforementioned timeline configuration method.
NB: the event filter is already exposed in the configuration and the event cache is now enabled by default.