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

Compare all args when testing for Overlapping Fields #521

Merged
merged 3 commits into from
Feb 5, 2020

Conversation

fortytw2
Copy link

This is primarily a fix for the way an old version of relay will sometimes arrange duplicate fragments (in my understanding... which may be wrong - I've just cherry picked this back onto master today)

We (myself and @bencallaway) have run this for almost a year now and are trying to get back onto upstream and port some of our changes back.

Without this change, this is an example of the error we encounter (apologies for the very blurry screenshot).

1571770211

@coveralls
Copy link

coveralls commented Oct 22, 2019

Coverage Status

Coverage decreased (-0.001%) to 92.376% when pulling 319cd87 on bookreport:updated-with-bens-fix into 59637ea on graphql-go:master.

@fortytw2 fortytw2 changed the title Compare all args when testing for overlap Compare all args when testing for Overlapping Fields Oct 22, 2019
@chris-ramon chris-ramon self-requested a review November 25, 2019 03:34
Copy link
Member

@chris-ramon chris-ramon left a comment

Choose a reason for hiding this comment

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

LGTM 👍 –– Thanks a lot @fortytw2, neat fix 💯 🚢

@chris-ramon chris-ramon merged commit c532893 into graphql-go:master Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants