-
Notifications
You must be signed in to change notification settings - Fork 90
feat_: kvstore service integration #17762
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
Conversation
Jenkins Builds
|
alexjba
left a comment
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 in general! 👍
Two minor questions
| result.QObject.setup | ||
|
|
||
| proc init*(self: Service) = | ||
| let response = status_kvstore.getStoreEntry() |
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.
all proc implementations of status_kvstore can throw. I think we could use some exceptions handling.
| text: qsTr("RPC statistics") | ||
| onClicked: rpcStatsModal.open() | ||
| } | ||
|
|
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.
What will this setting do for the user?
Adding @jorge-campo in case we need to update the documentation.
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.
It will be used to enable the RLN rate limit. Here are more detail: status-im/status-go#6475
|
Close it for logos-messaging/pm#277 (comment). |
What does the PR do
Use kvstore api provided by status-go
Affected areas
Architecture compliance
My PR is consistent with this document: Status Desktop Architecture Guide
Screenshot of functionality (including design for comparison)