-
Notifications
You must be signed in to change notification settings - Fork 96
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
Fix generating default values #1688
Fix generating default values #1688
Conversation
83f9b31
to
47c0a90
Compare
e0179ae
to
94584b9
Compare
assertFileContains(path + "api/DefaultApi.kt", | ||
"@PathVariable(name = \"apiVersion\", defaultValue = \"v5\") @Nullable apiVersion: BrowseSearchOrdersApiVersionParameter? = BrowseSearchOrdersApiVersionParameter.V5,", | ||
"@Header(name = \"Content-Type\", defaultValue = \"application/json\") @Nullable contentType: String? = \"application/json\"" | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last case I was hoping for is XFavorToken
default null to work. Is that easy to add?
@Header("X-Favor-Token") @Nullable xFavorToken: String? = null
Not sure you'd even have a defaultValue in the annotation for null since it has to be a string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Looking good, I got the same! Question about the default: null ones tho. @PathVariable(name = "apiVersion", defaultValue = "v5") @NotNull apiVersion: ApiVersion = ApiVersion.V5,
@Header("X-Favor-Token") @Nullable xFavorToken: String?,
@Header(name = "Content-Type", defaultValue = "application/json") @Nullable contentType: String? = "application/json" |
94584b9
to
c34358a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing Thanks! Default null worked in my test.
c34358a
to
872a460
Compare
Will this get into 6.12 snapshot soon? |
@altro3 can you merge 6.12.x into this branch? |
872a460
to
a801897
Compare
@sdelamo done |
@altro3 can you merge 6.12.x into this branch? |
a801897
to
1634754
Compare
Noticed 1 possible issue, but can open a new ticket #1686 (comment) |
Fixed #1686