-
Notifications
You must be signed in to change notification settings - Fork 258
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(media): some preparatory refactorings #4140
Conversation
…ding the content The tails of the prepare_attachment_message and prepare_encrypted_attachment_message were almost the same, with the one different that they were using different ctors for the `EventContent` types. In fact, all these `EventContent` types also expose a plain `new` function that can take in either an encrypted or a plain media source, so we can commonize the code there.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4140 +/- ##
==========================================
+ Coverage 84.72% 84.81% +0.09%
==========================================
Files 269 269
Lines 28779 28759 -20
==========================================
+ Hits 24384 24393 +9
+ Misses 4395 4366 -29 ☔ View full report in Codecov by Sentry. |
crates/matrix-sdk/src/media.rs
Outdated
let mut content = ImageMessageEventContent::plain(body, url).info(Box::new(info)); | ||
content.filename = filename; | ||
content.formatted = config.formatted_caption; |
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.
Could also use the assign!
macro here (I think I saw it still being used in a bunch of other places). Same for the other changes in this commit.
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.
Yep, make_attachment
in the latest version does maximally use assign!
, IMO 👍
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.
Nicely done.
…tion_id` This allows letting the caller whether they need to clone it or not, and avoids a spurious clone in one call site.
…caller The name wasn't very descriptive, and it's tweaking the content, so let's do that in place, instead of deferring to another method somewhere else in the codebase.
…tedFile` Changelog: Renamed `PrepareEncryptedFile` and `Client::prepare_encrypted_file` to `UploadEncryptedFile` and `Client::upload_encrypted_file`.
aka "Benjamin tries to make sense of the media code. Oh, nice: it makes sense."
Part of #1732.