Skip to content
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(sdk,ui): Remove RoomEventCacheUpdate::AddTimelineItems #4462

Merged
merged 5 commits into from
Jan 8, 2025

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Jan 6, 2025

A recent PR has introduced RoomEventCacheUpdate::UpdateTimelineEvents, see #4408. The reasons to add a variant instead of updating AddTimelineItems were the following:

  • smaller patches,
  • step by step work,
  • play and test in the apps during holidays,
  • if something went wrong, reverting would have been easier as it was only code additions.

But now it's time for the clean up! We have to remove the AddTimelineItems variant first, and then to remove the TimelineSettings::vectordiffs_as_inputs settings. Many tests have to be updated, and that's the majority of the changes brought by these patches.

Tasks:

  • Remove RoomEventCacheUpdate::AddTimelineItems
  • Remove TimelineSettings::vectordiffs_as_inputs

Hywan added 2 commits January 8, 2025 15:31
`owned_user_id` is only used by a test behind the
`experimental-sliding-sync` feature flag.
@Hywan Hywan force-pushed the chore-ui-timeline-cleanup branch 2 times, most recently from 5681b4e to 59afd8b Compare January 8, 2025 15:07
@Hywan Hywan marked this pull request as ready for review January 8, 2025 15:19
@Hywan Hywan requested a review from a team as a code owner January 8, 2025 15:19
@Hywan Hywan requested review from andybalaam and removed request for a team January 8, 2025 15:19
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Hywan added 3 commits January 8, 2025 16:35
This patch removes the `AddTimelineEvents` variant from
`RoomEventCacheUpdate` since it is replaced by `UpdateTimelineEvents`
which shares `VectorDiff`.

This patch also tests all uses of `UpdateTimelineEvents` in existing
tests.
From now on, this patch considers that `VectorDiff`s are the official
input type for the `Timeline`, via `RoomEventCacheUpdate` (notably
`::UpdateTimelineEvents`).

This patch removes `TimelineSettings::vectordiffs_as_inputs`. It thus
removes all deduplication logics, as it is supposed to be managed by the
`EventCache` itself.
@Hywan Hywan force-pushed the chore-ui-timeline-cleanup branch from 59afd8b to ba57d99 Compare January 8, 2025 15:35
@Hywan Hywan enabled auto-merge (rebase) January 8, 2025 15:51
@Hywan Hywan merged commit e19bdbf into matrix-org:main Jan 8, 2025
39 checks passed
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.32%. Comparing base (dc2775e) to head (ba57d99).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4462      +/-   ##
==========================================
+ Coverage   85.11%   85.32%   +0.20%     
==========================================
  Files         283      283              
  Lines       31790    31727      -63     
==========================================
+ Hits        27059    27071      +12     
+ Misses       4731     4656      -75     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants