Skip to content

Ordered results: + Document.OrderedResult integration#445

Closed
MartinKavik wants to merge 1 commit intoabsinthe-graphql:masterfrom
MartinKavik:fix-result-fields-order
Closed

Ordered results: + Document.OrderedResult integration#445
MartinKavik wants to merge 1 commit intoabsinthe-graphql:masterfrom
MartinKavik:fix-result-fields-order

Conversation

@MartinKavik
Copy link
Copy Markdown

@MartinKavik MartinKavik commented Nov 15, 2017

Issue #352
Well, this PR is quite big, but the most changes are new tests for ordered results.

  • The main goal was to add config option :ordered which switches between Document.Result and Document.OrderedResult
  • All tests are running on mix test

I can provide more info / write comments, etc. if you want. Thanks for review.

@benwilson512
Copy link
Copy Markdown
Contributor

Hey @MartinKavik I've created a PR on the absinthe_tutorial project that shows how you can accomplish this without any changes to Absinthe at all: absinthe-graphql/absinthe_tutorial#1

As noted in that PR, we may well make a few changes to Absinthe so that folks can avoid copying the whole Result module over, but other than that things are pretty straight forward. Let's continue the discussion on that PR.

@bruce
Copy link
Copy Markdown
Contributor

bruce commented Nov 22, 2017

@benwilson512 Should this be closed?

@bruce bruce changed the title WIP: Ordered results: + Document.OrderedResult integration Ordered results: + Document.OrderedResult integration Nov 22, 2017
@benwilson512
Copy link
Copy Markdown
Contributor

Yea, this isn't the way we want to go about this and it isn't feasible to try to redirect a PR this size.

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.

3 participants