-
Notifications
You must be signed in to change notification settings - Fork 20
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
add graphql data provider for data query #1038
Conversation
Maybe we need to close and reopen the PR to trigger CI |
.map_err(|e| Error::RequestError(format!("{:?}", e)))?; | ||
|
||
if let Some(value) = response.data.get("data") { | ||
let is_hodler_out: IsHodlerOut = serde_json::from_value(value.clone()).unwrap(); |
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.
need to make sure or add some comment that unwrap()
is much safe to use here, or return an Invalid data error.
Codecov Report
@@ Coverage Diff @@
## dev #1038 +/- ##
==========================================
- Coverage 20.62% 20.59% -0.04%
==========================================
Files 217 217
Lines 9060 9074 +14
==========================================
Hits 1869 1869
- Misses 7191 7205 +14
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
I have raised a few questions.
Are the older review comments resolved? If so, we should mark them as "resolved"
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!
Let's not forget we'd need multiple tests once the VC part is done
add new data provider graphql interface.
resolves #1030
resolves #1029
resolves #1024