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 method arguments. #166

Merged
merged 4 commits into from
Oct 31, 2018
Merged

Sort method arguments. #166

merged 4 commits into from
Oct 31, 2018

Conversation

grokys
Copy link
Collaborator

@grokys grokys commented Oct 30, 2018

A change was recently made to the GitHub GraphQL schema that made the arguments to GraphQL fields change. Asking the GraphQL team they said it's best not to rely on the order of arguments returned by the schema, so this PR adds code to sort arguments:

  • Args with no default value come first (requirement of C# language)
  • Then come paging args, sorted "first", "after", "last", "before"
  • All other args are sorted alphabetically

The args in the GitHub schema have changed order, and I've been told that relying on the order returned by the schema isn't a good idea, so improve our sorting of args:

- Args with no default value come first (requirement of C# language)
- Then come paging args, sorted  "first", "after", "last", "before"
- All other args are sorted alphabetically
@codecov
Copy link

codecov bot commented Oct 30, 2018

Codecov Report

Merging #166 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   87.95%   88.01%   +0.06%     
==========================================
  Files          73       73              
  Lines        3454     3472      +18     
==========================================
+ Hits         3038     3056      +18     
  Misses        416      416
Impacted Files Coverage Δ
Octokit.GraphQL.Core.Generation/EntityGenerator.cs 96.76% <100%> (ø) ⬆️
...okit.GraphQL.Core.Generation/InterfaceGenerator.cs 100% <100%> (ø) ⬆️
...raphQL.Core.Generation/Utilities/BuildUtilities.cs 100% <100%> (ø) ⬆️

@StanleyGoldman StanleyGoldman merged commit 5c5615d into master Oct 31, 2018
@StanleyGoldman StanleyGoldman deleted the fixes/sort-args branch October 31, 2018 11:24
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.

None yet

2 participants