Add missing directive definitions #3656
Conversation
|
|
||
| public static final String BOOLEAN = "Boolean"; | ||
| public static final String STRING = "String"; | ||
| public static final String NO_LONGER_SUPPORTED = "No longer supported"; |
There was a problem hiding this comment.
In more recent code, as a general rule we prefer to not have random strings hanging about. Some parts of this class haven't been changed in a long while and still have strings. If you prefer I can give the public static final String treatment to all the strings in this class
| .name("if") | ||
| .description(createDescription("Included when true.")) | ||
| .type(newNonNullType(newTypeName().name(BOOLEAN).build()).build()) | ||
| .build()) |
There was a problem hiding this comment.
A comment for myself: in AST land, DirectiveDefinitions ONLY have "inputValueDefinition", this is the AST type used to model "arguments". Confusingly there is another graphql.language/AST class called "Argument". It is meant to be inputValueDefinition here.
|
Do we need to declare the defer one as experimental? |
| public static final DirectiveDefinition SPECIFIED_BY_DIRECTIVE_DEFINITION; | ||
| @ExperimentalApi | ||
| public static final DirectiveDefinition ONE_OF_DIRECTIVE_DEFINITION; | ||
| @ExperimentalApi | ||
| public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; |
There was a problem hiding this comment.
Do we need to declare the defer one as experimental?
@andimarek it is declared here, did you also want the annotation to appear elsewhere?
Elsewhere all the defer codepaths have been annotated with @ExperimentalApi
Adds DirectiveDefinitions that were missing on built-in directives
@skip,@include, and@deferWe didn't previously have any tests for DirectiveDefinitions and I can see why, it's a definition rather than logic. But if you want tests I can add them.