-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
R4R: Move all store keys into constants #3119
Conversation
See comment @jackzampolin -- I'm not sure we should be doing this. |
@alexanderbez The strings don't provide objcap IIRC, it's because we use memory addresses in the store implementation. |
Codecov Report
@@ Coverage Diff @@
## develop #3119 +/- ##
===========================================
- Coverage 55.05% 54.35% -0.71%
===========================================
Files 133 137 +4
Lines 9430 10234 +804
===========================================
+ Hits 5192 5563 +371
- Misses 3921 4332 +411
- Partials 317 339 +22 |
Ahhh right! They are pointers! @jackzampolin can you please rebase? |
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.
Changes seem good! I left one remark on queriers 👍
Updated @alexanderbez |
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.
Simple enough changes and I don't see anything breaking, so LGTM. Warrants another review though.
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.
ACK
PENDING.md
with issue #Files changed
in the github PR explorerThis PR cleans up storekeys, query routes and message routes that were previously defined as string (e.g. "gov" and "acc") and used in many places in the SDK by placing them in their module.
cc @alexanderbez @cwgoes