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

ffi: Expose filename and formatted body fields for media captions #3171

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

SpiritCroc
Copy link
Contributor

@SpiritCroc SpiritCroc commented Feb 27, 2024

Enables clients to render media captions as per the recently merged MSC2530.

Related ruma PR: ruma/ruma#1732

  • Public API changes documented in changelogs (optional)

Signed-off-by: Tobias Büttner [email protected]

@SpiritCroc SpiritCroc requested a review from a team as a code owner February 27, 2024 08:27
@SpiritCroc SpiritCroc requested review from Hywan and removed request for a team February 27, 2024 08:27
@codecov-commenter
Copy link

codecov-commenter commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.84%. Comparing base (fd709b9) to head (46a5f0c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3171   +/-   ##
=======================================
  Coverage   83.84%   83.84%           
=======================================
  Files         232      232           
  Lines       24005    24005           
=======================================
+ Hits        20127    20128    +1     
+ Misses       3878     3877    -1     

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

@bnjbvr
Copy link
Member

bnjbvr commented Mar 8, 2024

Hi! Thanks for the PR, would you be willing to update Ruma as well?

@SpiritCroc
Copy link
Contributor Author

Hi! Thanks for the PR, would you be willing to update Ruma as well?

Maybe I'll have a try at the weekend (I'll probably have a look at updating this PR to prepare for ruma/ruma#1738 at some point either way), but if I remember correctly from last week, there were some compile issues with code I wasn't familiar with, so may be easier for someone else. Since I'm still somewhat of a Rust noob, I'm not planning to spend too much time on it, when it's likely that someone with better knowledge of the codebase will do that at some later point either way.

@SpiritCroc
Copy link
Contributor Author

SpiritCroc commented Mar 10, 2024

Looks like someone already updated ruma, so now the PR should already compile - just rebased it onto main to let the CI test.

@Hywan
Copy link
Member

Hywan commented Mar 11, 2024

Thanks for the PR! I would prefer to wait on ruma/ruma#1738 to be merged before merging this one :-).

@Hywan
Copy link
Member

Hywan commented Mar 11, 2024

The PR looks sensible though and should not be impacted by ruma/ruma#1738.

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.

Thanks!

@bnjbvr bnjbvr merged commit 2520804 into matrix-org:main Mar 14, 2024
34 checks passed
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.

4 participants