Add optional flag to include deprecated fields #26
Conversation
|
@realdm this seems like exactly what you're looking at |
| else | ||
| doc << '*' | ||
| end | ||
| doc << ' @deprecated ' |
There was a problem hiding this comment.
GraphQL also has deprecated enum values; is the @deprecated annotation valid for them in java?
There was a problem hiding this comment.
this PR addresses deprecation for fields not for types for now, but maybe you are right we should do the same for types
There was a problem hiding this comment.
but if you are asking if this annotation applicable for enum values the answer yes.
| <% fields.each do |field| %> | ||
| <%= java_doc(field) %> | ||
| <%= java_annotations(field) %> | ||
| <%= java_annotations(field) -%> |
There was a problem hiding this comment.
do not generate new line in case if java_annotations(field) resolved to blank line
dylanahsmith
left a comment
There was a problem hiding this comment.
A couple of minor suggested changes, otherwise, LGTM
codegen/lib/graphql_java_gen.rb
Outdated
|
|
||
| def erb_for(template_filename) | ||
| erb = ERB.new(File.read(template_filename)) | ||
| erb = ERB.new(File.read(template_filename), 0, '-') |
There was a problem hiding this comment.
Why are you also changing the safe_level argument from nil to non-nil.
If safe_level is set to a non-nil value, ERB code will be run in a separate thread with $SAFE set to the provided level.
Seems like you should just use nil for that argument.
codegen/lib/graphql_java_gen.rb
Outdated
| end | ||
|
|
||
| if element.respond_to?(:deprecated?) && element.deprecated? | ||
| if (doc.length > 0) |
There was a problem hiding this comment.
The parenthesis are unnecessary and I think if doc.empty? is more readable.
MobileBuy SDK requires to include deprecated fields that by default excluded from generation.
This PR:
include_deprecatedflag by default it's set to falseDeprecatedannotation for deprecated fieldsinclude_deprecatedis set totrueincludes deprecated fields into generationExample of generated deprecated fields:
Closes #6