Update mustache style #90
Conversation
…rt is added by code if needed)
| @@ -402,14 +402,20 @@ open class KotlinGenerator : SharedCodegen() { | |||
| ): CodegenOperation { | |||
| val codegenOperation = super.fromOperation(path, httpMethod, operation, definitions, swagger) | |||
|
|
|||
| retrofitImport.get(codegenOperation.httpMethod)?.let { codegenOperation.imports.add(it) } | |||
| retrofitImport[codegenOperation.httpMethod]?.let { codegenOperation.imports.add(it) } | |||
There was a problem hiding this comment.
I'm not a big fan of this syntax [...]?.whatever when the returned type is optional. I generally prefer having a chain of functions. Was this suggested by the IDE?
There was a problem hiding this comment.
It was on the "learn kotlin the Jetbrains way" 🤣
I just accepted the IDE suggestion.
| if (codegenParameter.isFile) { | ||
| codegenOperation.imports.add("okhttp3.RequestBody") | ||
| // The generated Retrofit APIs use RequestBody and not File objects | ||
| codegenOperation.imports.remove("File") | ||
| } |
There was a problem hiding this comment.
I understand that you're wrapping the "okhttp3.RequestBody" here as otherwise you'll have a bunch of unused imports. On the other hand you're adding a new endpoint to junit-test that is not covered by a test. I'd suggest you add the new endpoint in a separate PR. Ideally we should tackle also the Multipart scenario that I'm pretty sure we don't cover.
There was a problem hiding this comment.
I'd rather keep the already existing endpoint, I'll try to see how expensive would be creating a test for it.
There was a problem hiding this comment.
Creating a realistic test for it seems going a bit beyond the scope of the PR, especially considering that I'm getting the feeling that files are not properly handled (#99 )
Going to offload the effort in a follow-up PR
The goal of this PR is to improve our mustache templates to allow generation of code that will pass untouched through our linters (ktlint will consider the code good-looking).
The PR introduces changes on:
Map<K,V>should have a space in order to have ktlint happy with itI do I know it works as we'd like to?
Changes into
samples/*are essentially not present (if we exclude the removal of one indentation level ofX-Operation-IDheader).Running
returns an empty diff, so pre-commit considers the generated code "good-looking"