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

Improve querying the network subgraph, deserializing result data etc. #60

Merged
merged 3 commits into from
Sep 29, 2023

Conversation

Jannis
Copy link
Collaborator

@Jannis Jannis commented Sep 29, 2023

Based on #59, will need to be rebased after that is merged.

I am working on breaking down #54 into smaller PRs. This is the first of these.

Leverage serde and serde_json more for serializing queries and
deserializing responses in particular. There are tools that go even
further with typing GraphQL query and response types (see
https://github.com/graphql-rust/graphql-client#getting-started) but my
feeling is that would be overkill here.
Theodus
Theodus previously approved these changes Sep 29, 2023
Copy link
Member

@Theodus Theodus left a comment

Choose a reason for hiding this comment

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

LGTM

common/src/allocations/monitor.rs Outdated Show resolved Hide resolved
Co-authored-by: Theo Butler <theodusbutler@gmail.com>
Copy link
Collaborator

@aasseman aasseman left a comment

Choose a reason for hiding this comment

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

This is great, thanks @Jannis 🙏 !

@aasseman aasseman added size:medium Medium p2 Medium priority type:refactor Changes not visible to users labels Sep 29, 2023
@Jannis Jannis merged commit 702d744 into main Sep 29, 2023
5 checks passed
@Jannis Jannis deleted the jannis/improve-network-subgraph branch September 29, 2023 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2 Medium priority size:medium Medium type:refactor Changes not visible to users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants