-
Notifications
You must be signed in to change notification settings - Fork 285
feat(crypto): Add new encrypt_and_send_custom_to_device
to the client
#4998
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4998 +/- ##
=======================================
Coverage 85.85% 85.85%
=======================================
Files 325 325
Lines 35937 35937
=======================================
+ Hits 30852 30853 +1
+ Misses 5085 5084 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d4c0e90
to
c27cdd2
Compare
c27cdd2
to
2e746c0
Compare
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.
Not reviewed in fully yet, but there's a bit to discuss.
crates/matrix-sdk/src/client/mod.rs
Outdated
#[cfg(feature = "e2e-encryption")] | ||
pub async fn encrypt_and_send_custom_to_device( | ||
&self, | ||
targets: Vec<&Device>, |
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.
We call it targets
here but devices
and ,
recipient_devices` in other places. Is there potential to make it more consistent or are there other reasons why its different?
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.
Alright, did a fuller review, there are some docs that are wrong and I left some nits, but I think this can soon go in as an experimental feature.
Thx for the reviews and comments. I have updated the PR to adress the comments. I also added a changelog entry |
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.
One last thing and I think it's good to merge.
Created a follow-up PR to extract the mock helpers with the other mock helpers, see #5005
Signed-off-by: