-
Notifications
You must be signed in to change notification settings - Fork 25
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
SessionStoreExt::compute_safety_number #139
Conversation
@gferon, when you'd review, have a look at the new clippy warning about SessionExt not being able to be made into an object. |
At this stage, since we use those functionalities everywhere, what about simply merging the traits? |
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.
LGTM, one minor comment that you can disregard if you think that's ok.
return uuid.to_string(); | ||
} | ||
unreachable!( | ||
"an address requires either a UUID or a E164 phone number" |
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.
technically since ServiceAddress
is public, you could build it in an invalid state... do we really want this? (I know we have a few panics here and there in the lib, but maybe we can try avoiding new ones?).
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.
Hmm, then I'd rather have the initializer set to private at some point in the future.
The reason we have an extension trait is because the other traits are in libsignal-client, no? |
Indeed, I just forgot about it 😂 |
Helper method to bundle all the necessary SessionStore requests into computation of the safety number. Maybe some day we should get rid of the magical constants in here. 5200, really?