Skip to content
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 client/spacings indentations #9853

Merged
merged 25 commits into from
Aug 2, 2021
Merged

Kotlin client/spacings indentations #9853

merged 25 commits into from
Aug 2, 2021

Conversation

noordawod
Copy link
Contributor

@noordawod noordawod commented Jun 27, 2021

There has been changes in the Kotlin template that wrecked the beautifully-generated code that I'm used to, plus removed few important utility functions for enums for no obvious reason.

Well, I brought these utility functions back and also added plenty of reserved keywords and operators that were missing from the Kotlin generator.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    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.
  • File the PR against the correct branch: master, 5.1.x, 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@jimschubert @dr4ke616 @karismann @Zomzog @andrewemery @4brunu @yutaka0m

{{#multiplatform}}@SerialName(value = "{{{vendorExtensions.x-base-name-literal}}}") @Required {{/multiplatform}}{{#isInherited}}override {{/isInherited}}{{>modelMutable}} `{{name}}`: {{#isArray}}{{#isList}}kotlin.collections.List{{/isList}}{{^isList}}kotlin.Array{{/isList}}<{{^items.isEnum}}{{^items.isPrimitiveType}}{{^items.isModel}}{{#kotlinx_serialization}}@Contextual {{/kotlinx_serialization}}{{/items.isModel}}{{/items.isPrimitiveType}}{{{items.dataType}}}{{/items.isEnum}}{{#items.isEnum}}{{classname}}.{{{nameInCamelCase}}}{{/items.isEnum}}>{{/isArray}}{{^isEnum}}{{^isArray}}{{{dataType}}}{{/isArray}}{{/isEnum}}{{#isEnum}}{{^isArray}}{{classname}}.{{{nameInCamelCase}}}{{/isArray}}{{/isEnum}}{{#isNullable}}?{{/isNullable}}{{#defaultValue}} = {{{defaultValue}}}{{/defaultValue}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone mistakenly removed default values for this data class. Brought back now.

Copy link
Contributor

@etremblay etremblay Oct 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may not have been by mistake. I just updated to version 5.2.1 and enum values are now initialized with unknown class.

I think {{{defaultValue}}} may not be well defined.

data class MyDataClass (

    @Json(name = "type")
    val type: MyDataClass.Type = TypeEnum.MOVE,

) : MyDataClass {

    /**
     * 
     *
     * Values: MOVE
     */
    enum class Type(val value: kotlin.String) {
        @Json(name = "MOVE") MOVE("MOVE");
    }
}

TypeEnum is not something that exists in the codebase. It should be MyDataClass.type.MOVE I guess

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably similar to #10244

Copy link
Contributor

@4brunu 4brunu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@noordawod I left some questions to you in the code review, could you please take a look at those? 🙂

@noordawod
Copy link
Contributor Author

Build failed for another reason: Could not resolve dependencies for project com.prokarma:pkmst-microservice:jar:1.0.0: Could not find artifact io.springfox:springfox-swagger2:jar:2.6.1-SNAPSHOT

@noordawod
Copy link
Contributor Author

@noordawod I left some questions to you in the code review, could you please take a look at those? 🙂

I believe all is taken care of now, @4brunu.

@4brunu
Copy link
Contributor

4brunu commented Jul 5, 2021

@noordawod Could you please solve the merge conflicts?

/**
* Converts the provided [data] to a [String] on success, null otherwise.
*/
fun encode(data: Any?): kotlin.String? = if (data is {{classname}}) "$data" else null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@noordawod could you please explain what is the use case for this utility?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused by me, but because decode() is introduced I wanted to compliment it with an encode(), I think it's a good practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ps: I use decode().

@4brunu
Copy link
Contributor

4brunu commented Jul 20, 2021

@noordawod When you have some time, could you please solve the merge conflicts? 🙂

@noordawod
Copy link
Contributor Author

@noordawod When you have some time, could you please solve the merge conflicts?

Yup, was on a holiday ;) I'll attend to this asap!

@4brunu
Copy link
Contributor

4brunu commented Jul 27, 2021

Sorry, when you have time :)

@noordawod
Copy link
Contributor Author

Sorry, when you have time :)

Synced and pushed now, let's hope everything passes.

@noordawod
Copy link
Contributor Author

Sorry, when you have time :)

Some tests aren't passing because of Could not find org.openapitools:openapi-generator-gradle-plugin:5.2.0-SNAPSHOT.

@4brunu
Copy link
Contributor

4brunu commented Aug 2, 2021

Looks good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants