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

Simplify our serialization logic #18

Merged
merged 8 commits into from
Mar 2, 2022
Merged

Simplify our serialization logic #18

merged 8 commits into from
Mar 2, 2022

Conversation

poljar
Copy link
Collaborator

@poljar poljar commented Jan 7, 2022

A lot of types in our serialization logic seem to be of questionable usage. We should revise our serialization logic and where possible remove helper structs and use serde directly, this include error types.

@poljar poljar force-pushed the simpler-pickling branch 3 times, most recently from 4d6c33a to fa562a8 Compare February 15, 2022 10:51
src/megolm/ratchet.rs Outdated Show resolved Hide resolved
@poljar poljar requested a review from dkasak February 28, 2022 11:04
@poljar poljar marked this pull request as ready for review February 28, 2022 11:04
Copy link
Member

@dkasak dkasak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a bunch of comments. In the interest of getting this out as soon as possible, if any of my suggestions here are hard to do right now, I think the only critical thing is the boxing of Curve25519SecretKey. The rest can be done in a separate clean up PR if need be.

src/megolm/ratchet.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/megolm/inbound_group_session.rs Show resolved Hide resolved
src/olm/account/mod.rs Show resolved Hide resolved
src/megolm/inbound_group_session.rs Show resolved Hide resolved
src/megolm/inbound_group_session.rs Show resolved Hide resolved
README.md Show resolved Hide resolved
src/types/curve25519.rs Show resolved Hide resolved
@poljar poljar force-pushed the simpler-pickling branch from 8afaf1f to 84e6be0 Compare March 1, 2022 11:45
@poljar poljar force-pushed the simpler-pickling branch from 84e6be0 to dc5b44f Compare March 1, 2022 11:52
@poljar poljar force-pushed the simpler-pickling branch from 078606e to 0bfec70 Compare March 1, 2022 13:13
@poljar poljar requested a review from dkasak March 2, 2022 08:16
Copy link
Member

@dkasak dkasak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. Pushed a commit tweaking doc comments.

@poljar poljar merged commit b6e30ba into main Mar 2, 2022
@poljar poljar deleted the simpler-pickling branch June 8, 2022 10:50
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.

2 participants