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

Implemented #8. Both @deprecated(reason: String), or an empty @deprec… #9

Merged
merged 1 commit into from
Apr 27, 2017

Conversation

acidbluebriggs
Copy link
Contributor

@deprecated directive can be used within the schema definition and the messages will now be available when a client introspects the schema for deprecated values. The README.md has also been updated to reflect the new addition.

…), or an empty @deprecated, directive can be used within the schema definition and the messages will now be available when a client introspects the schema for deprecated values. The README.md has also been updated to reflect the new addition.
NEWHOPE @doc(d: "Released in 1977"),
EMPIRE @doc(d: "Released in 1980"),
JEDI @doc(d: "Released in 1983"),
FANTOM @doc(d: "Released in 1999") @deprecated(reason: "Not worth referencing"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apottere
Copy link
Collaborator

It looks like this is only applied to enum values - we'll probably have to expand it in the future to work on fields/arguments.

@acidbluebriggs
Copy link
Contributor Author

acidbluebriggs commented Apr 27, 2017

It's on fields also. You can see it on this line.
I just didn't create a new entry in the readme for that example. I just kept in line with your @doc example.

I'm not 100% sure that arguments are valid deprecation targets. I thought when I read the spec earlier it stated only enums and fields. I'll double check.

According to: http://facebook.github.io/graphql/#sec-Deprecation

Deprecation

To support the management of backwards compatibility, GraphQL fields and enum values can indicate whether or not they are deprecated (isDeprecated: Boolean) and a description of why it is deprecated (deprecationReason: String).

I see no option in the main GraphQL-Java api to deprecate an argument.

I should have worked on some sort of test, but that would have been a lot of work I couldn't get in quickly enough as there are no client tests in there at the moment and I really don't know much about the groovy testing framework. I was looking for one that tested the @doc directive, but there wasn't one.

@apottere
Copy link
Collaborator

Ah, I totally missed that line, sorry. I also wasn't aware that arguments didn't support deprecation - when the introspection query comes back, arguments have keys for isDeprecated and deprecationReason but that's probably just from their parent class.

I can add a test that checks the generated schema for deprecation messages tomorrow.

Thanks!

@apottere apottere merged commit d80d06e into graphql-java-kickstart:master Apr 27, 2017
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