-
Notifications
You must be signed in to change notification settings - Fork 246
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
Conversation
Codecov ReportAttention:
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
☔ View full report in Codecov by Sentry. |
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.
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. 🎉
This started as "oh it looks like I can avoid using
Account
here and there" until there was only a single meaningful use inOlmMachine
. But what is anAccount
, other than aReadOnlyAccount
with extra capabilities and access to a store? By passing the store as a parameter to mostAccount
methods, those methods could be moved to theReadOnlyAccount
, resulting in their merging \o/Then I could rename
ReadOnlyAccount
toAccount
. The universe is a bit more in order.This can be reviewed commit by commit.
Part of #2624.