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

feat(event cache): don't replace a gap chunk by an empty items chunks #4423

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Dec 17, 2024

It was simpler to add a new function rather than trying to fiddle with replace_gap_at, especially in terms of the return type of this function. After a few back-and-forth, they're close enough that we could probably merge them together, if you have strong opinions about it.

This was the only place where we'd create an empty chunk, so with this, the storage doesn't keep empty chunks anymore \o/

Part of #3280.

@bnjbvr bnjbvr requested a review from Hywan December 17, 2024 13:03
@bnjbvr bnjbvr requested a review from a team as a code owner December 17, 2024 13:03
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 87.09677% with 4 lines in your changes missing coverage. Please review.

Project coverage is 85.41%. Comparing base (5d8ad3a) to head (1a5d217).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk-common/src/linked_chunk/mod.rs 85.18% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4423      +/-   ##
==========================================
+ Coverage   85.39%   85.41%   +0.02%     
==========================================
  Files         283      283              
  Lines       31473    31490      +17     
==========================================
+ Hits        26876    26898      +22     
+ Misses       4597     4592       -5     

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

Copy link
Member

@Hywan Hywan 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, thanks! I think it's fine to keep remove_gap_at and replace_gap_at separated.

crates/matrix-sdk/src/event_cache/room/events.rs Outdated Show resolved Hide resolved
@bnjbvr bnjbvr force-pushed the bnjbvr/dont-replace-gap-with-empty-chunk branch from 78d94fd to 1a5d217 Compare December 17, 2024 17:12
@bnjbvr bnjbvr enabled auto-merge (rebase) December 17, 2024 17:12
@bnjbvr bnjbvr merged commit 373709f into main Dec 17, 2024
39 checks passed
@bnjbvr bnjbvr deleted the bnjbvr/dont-replace-gap-with-empty-chunk branch December 17, 2024 17:30
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