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

Make the null value customizable [wip] #151

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

gausby
Copy link
Contributor

@gausby gausby commented Feb 9, 2018

This pull request aims to make the null value customisable, so we can support systems that use a different from 'null' value as its concept of null; systems such as Elixir, and perhaps protobuf.

So far I've carried the context through to the paths that seems to be relevant; the context now carry a key called null_value that eventually will be a configuration option. I've changed the null value to owl; it doesn't really make sense besides testing that a different atom can be used—the plan is to switch this to null when it has proved its point so we can get this into the system without causing too much trouble.

It should be said: I don't know much about this part of the code, so my approach has been poking around, adding log messages, and changing values…it seems to work for the most part (as of writing there is still some null values being propagated), but I am not sure it is the correct approach.

Work in progress—at least I know more about the graphql-execution-module than I did before.

I've disabled two the introspection test case in the dungeon: The
introspection schema needs some modifications to make this work.

I the assertions to check for the custom null value of 'owl'; this
should prove that it is possible to set the null value to any atom.
@jlouis
Copy link
Owner

jlouis commented Feb 15, 2018

Nice!

Can I get you to rebase this on top of the current master ? I'm trying to reorder the code base to be a bit more controlled around that branch.

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.

2 participants