Skip to content
This repository was archived by the owner on Jul 25, 2024. It is now read-only.

Executor refactoring#82

Merged
freshtonic merged 10 commits intographql-elixir:masterfrom
freshtonic:refactor/executor-return-values
May 1, 2016
Merged

Executor refactoring#82
freshtonic merged 10 commits intographql-elixir:masterfrom
freshtonic:refactor/executor-return-values

Conversation

@freshtonic
Copy link
Copy Markdown
Contributor

@freshtonic freshtonic commented Apr 23, 2016

This is ready for review and merge. Once this is done, I'll rebase the mutations code on top.

@freshtonic freshtonic force-pushed the refactor/executor-return-values branch from 213294f to fb7cae3 Compare April 23, 2016 06:31
@freshtonic
Copy link
Copy Markdown
Contributor Author

The Executor is ugly and needs further refactoring but this is a step that is necessary for correct error reporting. Anything that reports an error has to be able to create a new ExecutionContext with a modified errors list.

This will allow us to report errors from anywhere in the flow and
accumulate them in the ExecutionContext.
@freshtonic freshtonic force-pushed the refactor/executor-return-values branch from cc3f714 to f6027f9 Compare April 23, 2016 08:02
@joshprice
Copy link
Copy Markdown
Member

Great work! Since we're changing the primary interface for the lib, we should maintain compatibility functions and deprecate them for the next version.

This will also need the corresponding changes in plug.

This should also resolve #56 and #64 but needs testing. #65 should also pass now so lets pull that into this PR.

After new version of plug is released that uses the new execute
function, the old execute function will be deleted.
@freshtonic freshtonic merged commit 23269cd into graphql-elixir:master May 1, 2016
@freshtonic freshtonic deleted the refactor/executor-return-values branch May 1, 2016 05:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants