Skip to content

Conversation

zecakeh
Copy link
Collaborator

@zecakeh zecakeh commented Dec 21, 2024

Sometimes we can get the bytes directly, e.g. in Fractal we can get an image from the clipboard. It avoids to have to write the data to a temporary file only to have the data loaded back in memory by the SDK right after.

The first commit to accept any type that implements Into<String> for the filename is grouped here because it simplifies slightly the second commit.

Note that we could also use AttachmentSource in the other send_attachment APIs, on Room and RoomSendQueue, for consistency.

@zecakeh zecakeh requested a review from a team as a code owner December 21, 2024 15:37
@zecakeh zecakeh requested review from jmartinesp and removed request for a team December 21, 2024 15:37
Copy link

codecov bot commented Dec 21, 2024

Codecov Report

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

Project coverage is 85.05%. Comparing base (1480fad) to head (eef5bb3).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk-ui/src/timeline/futures.rs 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4451      +/-   ##
==========================================
- Coverage   85.06%   85.05%   -0.01%     
==========================================
  Files         283      283              
  Lines       31748    31766      +18     
==========================================
+ Hits        27005    27019      +14     
- Misses       4743     4747       +4     

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

@poljar poljar requested review from poljar and removed request for jmartinesp December 22, 2024 16:20
…chment filename

Since we need a String as final type, this can avoid to allocate a string if the provided type is already a String.

Signed-off-by: Kévin Commaille <[email protected]>
Sometimes we can get the bytes directly. It avoids to have to write the data to a temporary file only to have the data loaded back in memory right after.

Signed-off-by: Kévin Commaille <[email protected]>
@zecakeh zecakeh force-pushed the timeline-send-attachment-bytes branch from c47466a to 2a0973f Compare December 22, 2024 17:25
@zecakeh
Copy link
Collaborator Author

zecakeh commented Dec 22, 2024

Added changelog entries in the commits.

@bnjbvr bnjbvr requested review from bnjbvr and removed request for poljar January 6, 2025 09:50
Copy link
Member

@bnjbvr bnjbvr left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks!

@bnjbvr
Copy link
Member

bnjbvr commented Jan 6, 2025

Once the conflict with CHANGELOG.md is fixed, I'm happy to land this.

@zecakeh
Copy link
Collaborator Author

zecakeh commented Jan 6, 2025

I merged main into this branch to solve the conflict. Maybe you would have preferred a rebase to keep the history clean?

@bnjbvr
Copy link
Member

bnjbvr commented Jan 6, 2025

I merged main into this branch to solve the conflict. Maybe you would have preferred a rebase to keep the history clean?

Thanks. It's rather up to you:

  • I don't tend to merge PRs which contain merge commits as is, instead I'll squash-merge them.
  • but if there's only clean atomic commits, I usually rebase-and-merge

So unless you have a strong preference for rebase-and-merge here (in which case, yes, please keep a clean history), I'll squash-merge later 👍

@zecakeh
Copy link
Collaborator Author

zecakeh commented Jan 6, 2025

It's fine, you can merge as is 🙂 .

@bnjbvr bnjbvr merged commit 70fb789 into matrix-org:main Jan 6, 2025
40 checks passed
@zecakeh zecakeh deleted the timeline-send-attachment-bytes branch January 13, 2025 15:13
yostyle pushed a commit to tchapgouv/matrix-rust-sdk that referenced this pull request Apr 4, 2025
Sometimes we can get the bytes directly, e.g. in Fractal we can get an
image from the clipboard. It avoids to have to write the data to a
temporary file only to have the data loaded back in memory by the SDK
right after.

The first commit to accept any type that implements `Into<String>` for
the filename is grouped here because it simplifies slightly the second
commit.

Note that we could also use `AttachmentSource` in the other
`send_attachment` APIs, on `Room` and `RoomSendQueue`, for consistency.

---------

Signed-off-by: Kévin Commaille <[email protected]>
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