Skip to content

[Bug][Kotlin-client] Can now handle path param of type list#12244

Merged
wing328 merged 5 commits intoOpenAPITools:masterfrom
sjoblomj:kotlin_client_path_param_list_and_cleanup
May 4, 2022
Merged

[Bug][Kotlin-client] Can now handle path param of type list#12244
wing328 merged 5 commits intoOpenAPITools:masterfrom
sjoblomj:kotlin_client_path_param_list_and_cleanup

Conversation

@sjoblomj
Copy link
Copy Markdown
Contributor

In #11228 I fixed #7199. However, the fix was only for library okhttp, and it turns out that also volley and multiplatform suffer from the same problem. This PR fixes this bug.

The PR also contains some cleaning up of the generated code (which is what most of the updated sample files is about).

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.
  • 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 @wing328

Comment on lines 55 to 61
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Kotlin type systems makes sure that the parameters cannot be null. Completely redundant check.

@sjoblomj sjoblomj force-pushed the kotlin_client_path_param_list_and_cleanup branch from d0af34a to 5a4db53 Compare April 26, 2022 14:54
@sjoblomj sjoblomj requested a review from jimschubert as a code owner April 26, 2022 15:03
@sjoblomj sjoblomj marked this pull request as draft April 26, 2022 15:10
@sjoblomj sjoblomj force-pushed the kotlin_client_path_param_list_and_cleanup branch 6 times, most recently from 974115f to e6a10c8 Compare April 27, 2022 22:53
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

jvm-volley refuses to build without android.useAndroidX=true, even if generateRoomModels is false

Comment on lines 103 to 105
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simplifying code

Comment on lines 72 to 65
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bugfix (#7199) to handle that you can't use toString() on lists

Comment on lines 75 to 70
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bugfix (#7199) to handle that you can't use toString() on lists

@sjoblomj sjoblomj marked this pull request as ready for review April 27, 2022 23:04
@sjoblomj sjoblomj force-pushed the kotlin_client_path_param_list_and_cleanup branch 2 times, most recently from 61007cd to 6d51d75 Compare April 30, 2022 10:28
@sjoblomj sjoblomj force-pushed the kotlin_client_path_param_list_and_cleanup branch from 6d51d75 to dae5b33 Compare May 1, 2022 08:54
@wing328
Copy link
Copy Markdown
Member

wing328 commented May 4, 2022

LGTM. Thanks for the fix.

@wing328 wing328 merged commit 706791f into OpenAPITools:master May 4, 2022
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.

[BUG][kotlin] Path params with type: array doesn't work

2 participants