-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[Kotlin] Allow optional header parameters with Kotlin and retrofit2 #9306
[Kotlin] Allow optional header parameters with Kotlin and retrofit2 #9306
Conversation
…tlin and retrofit2
@shanselm-ergon could you please update the sample projects by running the following commands?
Thanks |
I'm sorry, must have accidentally rollbacked the changes. I added a commit for it, thanks for the really fast reply! |
{{#isHeaderParam}}@Header("{{baseName}}") {{{paramName}}}: {{{dataType}}}{{#required}}{{#defaultValue}} = {{{.}}}{{/defaultValue}}{{/required}}{{^required}}?{{#defaultValue}} = {{{.}}}{{/defaultValue}}{{^defaultValue}} = null{{/defaultValue}}{{/required}}{{/isHeaderParam}} |
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.
@shanselm-ergon Hey, one question, this {{#defaultValue}} = {{{.}}}{{/defaultValue}}
is the same as this {{#defaultValue}} = {{{defaultValue}}}{{/defaultValue}}
?
Just to make sure because I didn't knew that sintaxe.
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.
@shanselm-ergon sorry if I didn't communicate clearly, I was not saying your code was incorrect, I was just asking to make sure I understood the code, because I didn't knew that sintaxe, but if it was correct, there was nothing wrong with it 🙂
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.
Yes, all good. I took the code from the issue #8809. Unfortunately I'm also not an expert in mustache. But I did some testing and it looks like both options work, or better said produce the same output. In the mustache documentation I couldn't find a clear answer how the dot operator works. Therefore I switched back to the, at least for me, simpler syntax.
I also executed the scripts again without any diff.
For me, this version is more readable, that was the reason I committed it.
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.
Thanks for taking the time to create this PR. Let's wait for CI to see if passes.
@4brunu is there a way to trigger the CI again? The problem seams unrelated to me. |
Yes, close and reopen this PR |
CI Build passed now. |
…tlin and ret… (OpenAPITools#9306) * [Kotlin] [OpenAPITools#8809] Allow optional header parameters with Kotlin and retrofit2 * [Kotlin] [OpenAPITools#8809] Update sample client * [Kotlin] [OpenAPITools#8809] Replace mustache dot notation with more classic style * [Kotlin] [OpenAPITools#8809] Remove new line at the end of the mustache template
[Kotlin] [#8809] Allow optional header parameters with Kotlin and retrofit2
fixes #8809
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
,5.1.x
,6.0.x
@jimschubert (2017/09) ❤️, @dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03)