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!(timeline): simplify getting the RepliedToInfo + introduce Room::load_or_fetch_event() #4837

Merged
merged 7 commits into from
Mar 25, 2025

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Mar 24, 2025

Small refactoring which should help with #4835.

Also introduces a new way to retrieve an event from the event cache, or if it's missing, from the network.

@bnjbvr bnjbvr requested a review from stefanceriu March 24, 2025 16:24
@bnjbvr bnjbvr requested a review from a team as a code owner March 24, 2025 16:25
Copy link

codecov bot commented Mar 24, 2025

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.45%. Comparing base (f8236a8) to head (44882ed).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...rates/matrix-sdk-ui/src/timeline/controller/mod.rs 50.00% 1 Missing ⚠️
crates/matrix-sdk/src/room/edit.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4837      +/-   ##
==========================================
- Coverage   86.45%   86.45%   -0.01%     
==========================================
  Files         297      297              
  Lines       34625    34599      -26     
==========================================
- Hits        29936    29913      -23     
+ Misses       4689     4686       -3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bnjbvr bnjbvr force-pushed the bnjbvr/timeline-reply-simplification branch from cc29b97 to f057959 Compare March 24, 2025 17:06
bnjbvr added 6 commits March 25, 2025 10:50
…h the replied-to event content

This is a nice simplification, because this means that:

1. we use a single way to get the event (event-cache-or-network)
2. we don't have to reconstruct a `RoomMessageEventContent` from a
timeline's message, which seems a bit error prone
3. there's a single way to get the replied-to info for an event,
4. and that's actually independent of the timeline, so we can improve
the code for #4835
@bnjbvr bnjbvr force-pushed the bnjbvr/timeline-reply-simplification branch from f057959 to 44882ed Compare March 25, 2025 09:50
@bnjbvr bnjbvr enabled auto-merge (rebase) March 25, 2025 09:52
@bnjbvr bnjbvr removed the request for review from stefanceriu March 25, 2025 09:53
@bnjbvr bnjbvr merged commit fc8a6dc into main Mar 25, 2025
42 checks passed
@bnjbvr bnjbvr deleted the bnjbvr/timeline-reply-simplification branch March 25, 2025 10:06
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