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.

Sign up for GitHub

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

Jump to bottom

Implement _entities field resolution #3

Merged
merged 1 commit into from
Jul 8, 2021
Merged

Implement _entities field resolution #3

merged 1 commit into from
Jul 8, 2021

Conversation

kdawgwilk
Copy link
Collaborator

@kdawgwilk kdawgwilk commented Jul 7, 2021 •
edited
Loading

This finishes the implementation for the _entities field resolution by allowing a type to implement the :_resolve_reference field. This is optional and the fallback is to just forward the representation from the gateway as the parent.

@kdawgwilk kdawgwilk requested review from MrGrinst and kzlsakal July 7, 2021 00:06
@kdawgwilk kdawgwilk self-assigned this Jul 7, 2021
@kdawgwilk kdawgwilk force-pushed the kaden/entities_field branch 4 times, most recently from 5ca70f9 to 2e56e71 Compare July 8, 2021 00:34
@kdawgwilk kdawgwilk marked this pull request as ready for review July 8, 2021 00:37
@kdawgwilk kdawgwilk force-pushed the kaden/entities_field branch 2 times, most recently from 5a95fd4 to 8fbc22b Compare July 8, 2021 00:45
MrGrinst
MrGrinst reviewed Jul 8, 2021
View reviewed changes
MrGrinst
MrGrinst previously approved these changes Jul 8, 2021
View reviewed changes
enewey
enewey requested changes Jul 8, 2021
View reviewed changes
Copy link
Collaborator

@enewey enewey left a comment

Choose a reason for hiding this comment

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

Feel free to ignore the nitpicks about the unused variable assignments, but I think the typename atomizing in resolve_type needs to be addressed, unless I'm misunderstanding how it works.

@kdawgwilk kdawgwilk force-pushed the kaden/entities_field branch 2 times, most recently from c202f46 to 3cd37d0 Compare July 8, 2021 18:17
enewey
enewey previously approved these changes Jul 8, 2021
View reviewed changes
@kdawgwilk kdawgwilk force-pushed the kaden/entities_field branch 2 times, most recently from 22b5d8b to 6e91df7 Compare July 8, 2021 18:43
@kdawgwilk @enewey
Implement _entities field resolution
5eabdf0 
Apply suggestions from code review

Co-authored-by: Erich Newey <enewey@users.noreply.github.com>
@kdawgwilk kdawgwilk force-pushed the kaden/entities_field branch from 6e91df7 to 5eabdf0 Compare July 8, 2021 18:55
@kdawgwilk kdawgwilk merged commit 640a234 into main Jul 8, 2021
@kdawgwilk kdawgwilk deleted the kaden/entities_field branch July 8, 2021 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enewey enewey enewey approved these changes

@MrGrinst MrGrinst MrGrinst approved these changes

@kzlsakal kzlsakal Awaiting requested review from kzlsakal

Assignees

@kdawgwilk kdawgwilk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kdawgwilk @enewey @MrGrinst