-
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
Move libolm migration code to a separate crate #2959
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main matrix-org/matrix-rust-sdk#2959 +/- ##
==========================================
+ Coverage 83.50% 83.52% +0.02%
==========================================
Files 221 223 +2
Lines 23007 23121 +114
==========================================
+ Hits 19211 19311 +100
- Misses 3796 3810 +14 ☔ View full report in Codecov by Sentry. |
If that's experimental, I'm removing @bnjbvr from the reviewer list. Please, ping someone once it's ready for a review. |
8ff55db
to
e433a2d
Compare
... and change `migrate_data` to consume one.
... rather than a `ProgressListener`.
... rather than tying it to a SQLite store.
... because `parse_user_id` returns an FFI-specific error type.
35e7545
to
289e304
Compare
use serde::{Deserialize, Serialize}; | ||
|
||
/// Struct collecting data that is important to migrate sessions to the rust-sdk | ||
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] |
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.
This is a simple move of these 5 types, with the only difference being that the uniffi:*
derivation needs to become conditional on the feature flag.
@@ -121,7 +121,7 @@ where | |||
let tracked_users: Vec<_> = data | |||
.tracked_users | |||
.into_iter() | |||
.filter_map(|s| UserId::parse(&s).ok().map(|u| (u, true))) | |||
.filter_map(|s| UserId::parse(s).ok().map(|u| (u, true))) |
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.
Seems like we run the clippy checks on the new crate but not the old one?
I've decided not to do this. It turns out that we can't actually share much code with the wasm bindings anyway, because the wasm bindings want their own wrapper classes. Plus there are various weirdnesses in the existing code which make them really horrible to use, but fixing them would break apps using the FFI bindings (notably: timestamps which are strings holding seconds-since-the-epoch: #2974). |
This is currently something of an experimental PR, but I want to run the CI checks on it.
Part of matrix-org/matrix-rust-sdk-crypto-wasm#78.