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

Sort fields before printing to get a more stable diff #889

Merged
merged 3 commits into from
May 30, 2017
Merged

Sort fields before printing to get a more stable diff #889

merged 3 commits into from
May 30, 2017

Conversation

tberman
Copy link
Contributor

@tberman tberman commented May 27, 2017

Sort the fields of an object similar to how types are being sorted.

@@ -585,7 +585,7 @@ describe('Subscribe', () => {
invalidEmailSchema,
ast
);
}).to.throw('Subscription must return AsyncIterable. Received: test');
}).to.throw('Subscription must return Async Iterable. Received: test');
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing.

@wincent
Copy link
Contributor

wincent commented May 30, 2017

Thnaks @tberman.

@wincent wincent merged commit f5dd099 into graphql:master May 30, 2017
@leebyron
Copy link
Contributor

leebyron commented Jun 20, 2017

Unfortunately this PR broke how this utility is used for printing schema in tests and tools - the sorted behavior changed which fields appear first which some tools rely on.

I'm reverting this change, I suggest if stable diffs are required via field sorting, that fields are sorted while still in the schema rather than during printing of the schema. Otherwise the defined order of fields is stable and should be kept stable by this function as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants