Skip to content
This repository has been archived by the owner on Oct 26, 2020. It is now read-only.

Support for repeatable directives #5

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

travisbrown
Copy link

This PR includes Oleg's work from sangria-graphql#412 merged into current master (which mostly involved replacing Unicode arrows). It closes this issue.

@jregistr
Copy link

jregistr commented Nov 2, 2019

This PR was made before the 2.13 PRs. I'm seeing in there the little arrows for example. Were you thinking this should go in supporting the non 2.13 version of this library or should we update this PR for 2.13? Or are the two not mutually exclusive?

@travisbrown
Copy link
Author

@jregistr I just missed some of the arrows when I merged master. I've pushed another commit replacing the rest (everywhere but in the CHANGELOG).

@yanns
Copy link

yanns commented Nov 5, 2019

By looking at the description of the original PR, Oleg wrote that we can merge this PR as soon as graphql/graphql-js#1541 is merged.
But the later one is still opened.
So I'm wondering if we should merge this PR.

@yanns
Copy link

yanns commented Nov 5, 2019

The PR that was merged (graphql/graphql-js#1965) does not change the introspection.
I personally don't really know how to handle this PR.

@travisbrown
Copy link
Author

@yanns Should we remove the introspection changes (essentially the same operation as between those two PRs)?

@jregistr
Copy link

I would vote to keep the introspection even if that makes it different than the reference js implementation. Besides this, aside from rebasing for conflict resolution; what else is left to merge this PR?

@yanns
Copy link

yanns commented Nov 27, 2019

@jregistr can you please elaborate why you think we should merge this as it is?

@yanns yanns force-pushed the master branch 5 times, most recently from bf1dfe9 to 1e0131b Compare December 23, 2019 11:12
@yanns yanns removed this from the 2.0.0 milestone Apr 16, 2020
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.

Support for repeatable directives
4 participants