swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.93k stars 6.03k forks source link

[C++][Qt5] Rewrite of Qt5 Code Generator #5521

Open tbe opened 7 years ago

tbe commented 7 years ago

The Code generated by the qt5cpp generator is incomplete and does not really match the other client library structures ( like cpprest, perl, ... ).

I would create a complete new set of templates and adapt the generator code, to make sure, the Qt5CPP generator behaves like cpprest and is more feature complete.

Is there any active maintainer for Qt5 or such a project already in progress?

As there is also #3203 outstanding for Qt5 i would also take care of this.

wing328 commented 7 years ago

@tbe thanks for offering help to improve the Qt5 C++ generator.

I would suggest you to base the enhancements on 2.3.0 (for breaking changes)

I've copied the contributors of Qt5 C++ generator below:

@Ragnis @fehguy @sabras75 @philicious @fortiumant @marache

Ref: https://github.com/swagger-api/swagger-codegen/pulls?utf8=%E2%9C%93&q=is%3Apr%20label%3A%22Client%3A%20C%2B%2B%22%20is%3Aclosed%20QT5

marache commented 7 years ago

Hi @tbe, we have a current WIP version of a closer to Qt5 way of doing things, I'll try to propose this has a Pull Request linked to this issue. It might be a great way of starting a discussion ?

tbe commented 7 years ago

Hi @marache , that would be great.

@wing328, @marache i think, using QNetworkAuth for handling OAuth/OAuth2 as an optional feature would be a good idea. Any thoughts about this?

We could just ignore the authentication key handling for OAuth/OAuth2 based services and allow the user either to handle the complete authentication flow with the generated code, or just supply the authkey to the underlying QAbstractOAuth2 class.

marache commented 7 years ago

It's not a PR yet as we did a new plugin, I pushed the latest version here : https://github.com/cstb/qt5-swagger-codegen

wing328 commented 7 years ago

i think, using QNetworkAuth for handling OAuth/OAuth2 as an optional feature would be a good idea. Any thoughts about this?

Never use it myself. After reading the doc, I think we should give it a try đź‘Ť

tbe commented 7 years ago

I'll take the code from @marache as base, integrate the authentication handling, better namespace support ( like cpprest ) and apply the google code style where needed and possible.

wing328 commented 7 years ago

@tbe just want to let you know that we target to release 2.3.0 by the end of July (postponed from end of May earlier)

If the refactor is ready, we'll include it in the 2.3.0 release. (this is not to say we must include the refactored Qt5 c++ client in the 2.3.0 release so please take your time with the factoring and let us know if there's anything we can help)

wing328 commented 7 years ago

FYI. We've started publishing a SNAPSHOT version of current master (2.2.3) and 2.3.0 (a branch with breaking changes). Please refer to https://github.com/swagger-api/swagger-codegen#compatibility for more information.

antis81 commented 6 years ago

Once again I took the rocky road and went through closing all those nasty mem-leaks and thread leaks manually in the generated code base. Thanks to valgrind this is a doable task in hours instead of days… However there's not much needed to fix the default templates for good and make them "just work".

antis81 commented 6 years ago

Do we have a 2.2.x branch where I can request the fix to be merged? It's not completely finished/tested, code is here -> antis81/patch-1

wing328 commented 6 years ago

@antis81 please file against 2.3.0 branch, which will be released by the end of this month.

antis81 commented 6 years ago

@wing328 I couldn't find a 2.3.x branch to file against. Do you mean master?

wing328 commented 6 years ago

@antis81 right, I'm referring to the master (which will be released as 2.3.0). Sorry for any confusion.

antis81 commented 6 years ago

Alright, rebased the changes onto master and pushed. Not sure everything works as I cannot actually test it. Could someone review or even test it further? (will copy the technical committee ^^) Am just about to open the PR and we see further… :)