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

crypto: Fuse ReadOnlyAccount and Account #2680

Merged
merged 9 commits into from
Oct 9, 2023
Merged

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Oct 6, 2023

This started as "oh it looks like I can avoid using Account here and there" until there was only a single meaningful use in OlmMachine. But what is an Account, other than a ReadOnlyAccount with extra capabilities and access to a store? By passing the store as a parameter to most Account methods, those methods could be moved to the ReadOnlyAccount, resulting in their merging \o/

Then I could rename ReadOnlyAccount to Account. The universe is a bit more in order.

This can be reviewed commit by commit.

Part of #2624.

@bnjbvr bnjbvr requested a review from poljar October 6, 2023 16:25
@bnjbvr bnjbvr requested a review from a team as a code owner October 6, 2023 16:25
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Attention: 36 lines in your changes are missing coverage. Please review.

Comparison is base (6c9cfe1) 78.18% compared to head (71861d9) 78.16%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2680      +/-   ##
==========================================
- Coverage   78.18%   78.16%   -0.03%     
==========================================
  Files         191      191              
  Lines       19851    19831      -20     
==========================================
- Hits        15521    15501      -20     
  Misses       4330     4330              
Files Coverage Δ
crates/matrix-sdk-crypto/src/dehydrated_devices.rs 100.00% <100.00%> (ø)
crates/matrix-sdk-crypto/src/gossiping/machine.rs 65.26% <100.00%> (ø)
crates/matrix-sdk-crypto/src/identities/device.rs 75.48% <100.00%> (ø)
crates/matrix-sdk-crypto/src/identities/manager.rs 81.94% <100.00%> (-0.25%) ⬇️
crates/matrix-sdk-crypto/src/identities/user.rs 86.54% <ø> (ø)
crates/matrix-sdk-crypto/src/lib.rs 100.00% <ø> (ø)
crates/matrix-sdk-crypto/src/machine.rs 83.24% <100.00%> (-0.40%) ⬇️
...atrix-sdk-crypto/src/olm/group_sessions/inbound.rs 82.42% <ø> (ø)
...trix-sdk-crypto/src/olm/group_sessions/outbound.rs 93.51% <ø> (ø)
crates/matrix-sdk-crypto/src/olm/signing/mod.rs 88.72% <ø> (ø)
... and 13 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

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

Thanks for splitting this out. Was reasonably pleasant to review because of this. Another thanks for realizing that we don't need both of them anymore. 🎉

@bnjbvr bnjbvr merged commit 2a78b92 into main Oct 9, 2023
36 checks passed
@bnjbvr bnjbvr deleted the simplify-userid-deviceid branch October 9, 2023 09:00
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