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

[Dart 2] Add support for Dart 2 #754

Merged
merged 16 commits into from Aug 31, 2018

Conversation

Projects
None yet
3 participants
@swipesight
Contributor

swipesight commented Aug 7, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
    @ircecho

Description of the PR

Added an config option for Dart 2.

@swipesight swipesight closed this Aug 7, 2018

@swipesight swipesight reopened this Aug 7, 2018

@wing328

This comment has been minimized.

Show comment
Hide comment
@wing328

wing328 Aug 7, 2018

Member

@swipesight thanks for the PR. Is it correct to say that this will break users using Dart 1.x? Shall we add an option e.g. supportDart2?

I believe most Dart developers should still be using Dart 1.x as Dart 2.0.0 (stable version) was only released yesterday/today.

Member

wing328 commented Aug 7, 2018

@swipesight thanks for the PR. Is it correct to say that this will break users using Dart 1.x? Shall we add an option e.g. supportDart2?

I believe most Dart developers should still be using Dart 1.x as Dart 2.0.0 (stable version) was only released yesterday/today.

@swipesight

This comment has been minimized.

Show comment
Hide comment
@swipesight

swipesight Aug 7, 2018

Contributor

@wing328 How about adding a new generator for Dart2?

Contributor

swipesight commented Aug 7, 2018

@wing328 How about adding a new generator for Dart2?

@wing328

This comment has been minimized.

Show comment
Hide comment
@wing328

wing328 Aug 7, 2018

Member

@swipesight that's one way to do it but I would a prefer a single generator for Dart if Dart 1.x and Dart 2.x are not very different.

I believe Dart2 can still use Dart 1.x library, right? In other words, we don't need to update the dependency.

Member

wing328 commented Aug 7, 2018

@swipesight that's one way to do it but I would a prefer a single generator for Dart if Dart 1.x and Dart 2.x are not very different.

I believe Dart2 can still use Dart 1.x library, right? In other words, we don't need to update the dependency.

@swipesight

This comment has been minimized.

Show comment
Hide comment
@swipesight

swipesight Aug 7, 2018

Contributor

@wing328 I'll see if I can add an option for Dart2.

Contributor

swipesight commented Aug 7, 2018

@wing328 I'll see if I can add an option for Dart2.

@wing328

This comment has been minimized.

Show comment
Hide comment
@wing328

wing328 Aug 7, 2018

Member

@swipesight 👌 let me know if you need any help.

Python flask generator has something similar: supportPython2 option: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonFlaskConnexionServerCodegen.java#L43

Search for supportPython2 or SUPPORT_PYAHON2 will reveal how the option is implemented in that file.

Hope this helps.

Member

wing328 commented Aug 7, 2018

@swipesight 👌 let me know if you need any help.

Python flask generator has something similar: supportPython2 option: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonFlaskConnexionServerCodegen.java#L43

Search for supportPython2 or SUPPORT_PYAHON2 will reveal how the option is implemented in that file.

Hope this helps.

@wing328 wing328 modified the milestones: 3.2.1, 3.2.2 Aug 14, 2018

@swipesight swipesight changed the title from [Dart 2] Constants in the core libraries have been renamed from SCREA… to [Dart 2] Support Dart 2 Aug 17, 2018

@swipesight swipesight changed the title from [Dart 2] Support Dart 2 to [Dart 2] Add support for Dart 2 Aug 18, 2018

@swipesight

This comment has been minimized.

Show comment
Hide comment
@swipesight

swipesight Aug 18, 2018

Contributor

@wing328 I have added an option for Dart 2.

Contributor

swipesight commented Aug 18, 2018

@wing328 I have added an option for Dart 2.

@wing328 wing328 modified the milestones: 3.2.2, 3.2.3 Aug 22, 2018

wing328 added some commits Aug 24, 2018

@wing328

This comment has been minimized.

Show comment
Hide comment
@wing328

wing328 Aug 24, 2018

Member

@swipesight I tried to add Dart2 petstore samples to the Travis build but it reports the following error:

Resolving dependencies...
The current Dart SDK version is 2.0.0.
Because petstore_client depends on openapi from path which requires SDK version <2.0.0, version solving failed.
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (pub-get) on project Dart2PetstoreClientTests: Command execution failed.: Process exited with an error: 1 (Exit value: 1) -> [Help 1]

Would you please take a look when you've time?

Basically, the test runs the following in the folder samples/client/petstore/dart2/petstore/:

pub get
pub build test
Member

wing328 commented Aug 24, 2018

@swipesight I tried to add Dart2 petstore samples to the Travis build but it reports the following error:

Resolving dependencies...
The current Dart SDK version is 2.0.0.
Because petstore_client depends on openapi from path which requires SDK version <2.0.0, version solving failed.
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (pub-get) on project Dart2PetstoreClientTests: Command execution failed.: Process exited with an error: 1 (Exit value: 1) -> [Help 1]

Would you please take a look when you've time?

Basically, the test runs the following in the folder samples/client/petstore/dart2/petstore/:

pub get
pub build test
@swipesight

This comment has been minimized.

Show comment
Hide comment
@swipesight

swipesight Aug 26, 2018

Contributor

@wing328 Sure, I'll check out later.

Contributor

swipesight commented Aug 26, 2018

@wing328 Sure, I'll check out later.

@Henge9

This comment has been minimized.

Show comment
Hide comment
@Henge9

Henge9 Aug 29, 2018

This PR solves version issues when upgrading to latest flutter version.

Henge9 commented Aug 29, 2018

This PR solves version issues when upgrading to latest flutter version.

@wing328 wing328 modified the milestones: 3.2.3, 3.3.0 Aug 30, 2018

@wing328

This comment has been minimized.

Show comment
Hide comment
@wing328

wing328 Aug 31, 2018

Member

@swipesight I've pushed 7d2edf4 to move the Dart2 petstore test to the end as some records created by the Dart2 petstore tests are invalid (e.g. status not set probably). I'll follow up with you later to fix those test cases.

If the CI tests are green, I'll merge this PR into master

Member

wing328 commented Aug 31, 2018

@swipesight I've pushed 7d2edf4 to move the Dart2 petstore test to the end as some records created by the Dart2 petstore tests are invalid (e.g. status not set probably). I'll follow up with you later to fix those test cases.

If the CI tests are green, I'll merge this PR into master

@swipesight

This comment has been minimized.

Show comment
Hide comment
@swipesight

swipesight Aug 31, 2018

Contributor

@wing328 I'll fix these tests and then send another PR.

Contributor

swipesight commented Aug 31, 2018

@wing328 I'll fix these tests and then send another PR.

@wing328 wing328 merged commit d327c5b into OpenAPITools:master Aug 31, 2018

4 checks passed

Shippable Run 2671 status is SUCCESS.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wing328

This comment has been minimized.

Show comment
Hide comment
@wing328

wing328 Aug 31, 2018

Member

@swipesight thanks! We'll send out a tweet to promote the awesome Dart2 support you added.

Do you have a Twitter account? If yes, we'll tag you in our tweet (https://twitter.com/oas_generator)

If you need help fixing the tests, please let me know.

Member

wing328 commented Aug 31, 2018

@swipesight thanks! We'll send out a tweet to promote the awesome Dart2 support you added.

Do you have a Twitter account? If yes, we'll tag you in our tweet (https://twitter.com/oas_generator)

If you need help fixing the tests, please let me know.

@swipesight

This comment has been minimized.

Show comment
Hide comment
@swipesight

swipesight Aug 31, 2018

Contributor

@wing328 Thanks! Here is my twitter https://twitter.com/swipesight

Contributor

swipesight commented Aug 31, 2018

@wing328 Thanks! Here is my twitter https://twitter.com/swipesight

@swipesight swipesight referenced this pull request Sep 2, 2018

Merged

[Dart 2] Fix petstore tests #954

4 of 4 tasks complete
@wing328

This comment has been minimized.

Show comment
Hide comment
@wing328

wing328 Sep 2, 2018

Member

Set Dart2 as the default via #949

Member

wing328 commented Sep 2, 2018

Set Dart2 as the default via #949

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