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] Ktor server upgrade (v1.5.2) #9088

Merged
merged 3 commits into from
Apr 28, 2021

Conversation

rsinukov
Copy link
Contributor

@rsinukov rsinukov commented Mar 25, 2021

#9087

Hi,
I am part of the Ktor team.
This PR updates Ktor server generator to the newest version of Kotlin and Ktor. Changes:

  • Add flag to choose between using Locations feature or default routing
  • Generate locations for get requests too
  • Update to the latest version and fix warnings and errors
  • Minor codestyle fixes

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:
    ...
  • 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]
    @jimschubert (2017/09), @dr4ke616 (2018/08) @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03)

@wing328
Copy link
Member

wing328 commented Mar 29, 2021

@rsinukov thanks for the PR. Is it a breaking change with or without fallback?

@rsinukov
Copy link
Contributor Author

I am not sure what exactly you mean by breaking change. It will generate projects with updated dependencies and update usage of Ktor to the newest API. So that generated code will be changed and users will need to update their code. In terms of the generator itself, I added one flag, that has the default value to support previous behaviour. So no breaking change here, only additive.

@wing328
Copy link
Member

wing328 commented Mar 30, 2021

So that generated code will be changed and users will need to update their code.

Looks like a breaking change as the users will need to update their code accordingly so this change should target 6.0.x branch instead.

I wonder if the new feature featureLocations can be filed by another PR separately or it has a dependency on the latest version of Ktor

@rsinukov
Copy link
Contributor Author

I wonder if the new feature featureLocations can be filed by another PR separately or it has a dependency on the latest version of Ktor

It can be in separate PR in theory, but to make it work as it is designed, it would still be a breaking change.

@rsinukov rsinukov changed the base branch from master to 6.0.x March 30, 2021 10:58
@wing328
Copy link
Member

wing328 commented Apr 28, 2021

To move it forward, I will include it in the upcoming "minor" release 5.2.0 and created kotlin-server-deprecated generator in case users want to stick with the current version/output.

I'll merge this after the 5.1.1 release due shortly.

@rsinukov rsinukov changed the base branch from 6.0.x to 5.2.x April 28, 2021 10:46
@rsinukov
Copy link
Contributor Author

Ok, I'll update PR and will keep old version in kotlin-server-deprecated

@wing328
Copy link
Member

wing328 commented Apr 28, 2021

Please hold on ...

@wing328 wing328 added this to the 5.1.1 milestone Apr 28, 2021
@wing328 wing328 merged commit 53398a0 into OpenAPITools:5.2.x Apr 28, 2021
@wing328
Copy link
Member

wing328 commented Apr 28, 2021

Merged. Please file a PR to create kotlin-server-deprecated later when you've time.

@wing328
Copy link
Member

wing328 commented Apr 28, 2021

@rsinukov when you've time, please also review #9358 when you've time and let the author your opinion on the change.

@rsinukov
Copy link
Contributor Author

Resolves #9087

@rsinukov
Copy link
Contributor Author

@wing328 I just opened PR with deprecated version of the generator #9365

@wing328 wing328 modified the milestones: 5.1.1, 5.2.0 Apr 29, 2021
@wing328 wing328 changed the title Ktor server upgrade Ktor server upgrade (v1.5.2) Apr 29, 2021
@wing328 wing328 changed the title Ktor server upgrade (v1.5.2) [kotlin] Ktor server upgrade (v1.5.2) Apr 29, 2021
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.

2 participants