-
Notifications
You must be signed in to change notification settings - Fork 269
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
MSC2530: added the ability to send media with captions #3226
Conversation
In relevance to MSC2530
Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3226 +/- ##
==========================================
- Coverage 83.67% 83.64% -0.03%
==========================================
Files 236 236
Lines 24398 24415 +17
==========================================
+ Hits 20414 20421 +7
- Misses 3984 3994 +10 ☔ View full report in Codecov by Sentry. |
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.
Just a quick review of things that jumped out at me.
The comments usually apply to several identical parts of the code.
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.
Wow, that's really nice, thanks for working on this! I'm way too excited about getting captioned media in all apps 👀
There are a few CI tasks failing. To re-run them at home:
- clippy:
cargo xtask ci clippy
- documentation task:
cargo test --doc --features docsrs
Happy to take another look when the comments I've posted have been addressed 😊
All green 🎉 |
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.
Getting there, thanks for the changes! Only a few comments about doc comments, and we should be good to go.
Thanks! Can you edit the original message to include the signoff, as requested in our contribution guidelines, please? |
(CI failure is unrelated.) |
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.
Thanks a bunch 🙏 Exciting!
Yay! 🎉 |
Now that there is some support for MSC2530, I gave adding sending captions a try. ( This is my first time with Rust 😄 )
I tried it on Element X with a hardcoded caption and it seems to work well
(It even got forwarded through mautrix-whatsapp and the caption was visible on the Whatsapp side)
Signed-off-by: Marco Alvarez surakin@gmail.com