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

New generator swift5 #4086

Merged
merged 34 commits into from Jan 2, 2020
Merged

New generator swift5 #4086

merged 34 commits into from Jan 2, 2020

Conversation

@4brunu
Copy link
Contributor

4brunu commented Oct 7, 2019

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, ./bin/openapi3/{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\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

@jgavris (2017/07) @ehyche (2017/08) @Edubits (2017/09) @jaz-ah (2017/09) @d-date (2018/03)

Description of the PR

This PR adds basic support for Swift 5 and it lowers the barrier of any Swift 5 contribution.
It's basically a copy of the Swift 4 generator, but it adds support for Swift Package Manager (fix #4057), updates the dependencies version.

EDIT:
I have updated this PR to introduce some new features in along side with the new Swift 5 generator.
The new features that the Swift 5 generator has, compared with the Swift 4 generator are:

  • Add nullable (or x-nullable in OAS2) support. This will replace the unwrapRequired option.
  • Add support for URLSession
  • Date formatter add support for ISO8601 with and without milliseconds (fixes #699 and it's a continuation of #1454)
  • make enums conform to CaseIterable (fixes #4842)
  • Add support for Combine framework
  • cocoapods remove deprecated docset_url and add watchos deployment target
  • Add ability to pass in a dedicated queue for processing network response (Fix for #230)

This PR fixes #2721

@d-date

This comment has been minimized.

Copy link
Contributor

d-date commented Oct 7, 2019

@4brunu Thanks for contribution but we plan to make another generator which reduce dependencies. #4086

Since Xcode 11 has officially released so I restart to work on. Sorry for late work.

@4brunu

This comment has been minimized.

Copy link
Contributor Author

4brunu commented Oct 7, 2019

@d-date No problem 🙂
Can you elaborate on which dependencies are you removing? Is is Alamofire or another one?
Looking forward to it.
Do you have an idea on when are you planing to finish it?
Should I delete this PR?

@wing328

This comment has been minimized.

Copy link
Member

wing328 commented Oct 8, 2019

Like other generators such as Java, Ruby that are more mature, I foresee Swift 5 generators will support the library option for different HTTP libraries so my take is to accept this one before anyone has time to add the library option for this Swift 5 generator.

@d-date

This comment has been minimized.

Copy link
Contributor

d-date commented Oct 8, 2019

@wing328 I don't agree with you. IMO, all dependencies should be removed since no more maintain so many templates for response types and APIClients.
We should provide only interfaces and models.

@d-date

This comment has been minimized.

Copy link
Contributor

d-date commented Oct 8, 2019

@4brunu I'm trying to remove all dependencies including Alamofire since this client shouldn't have any dependencies. We want to select own API Clients and response type, including Combine, by myself.

Instead, in other repository, we should provide example API Client for users so we can focus on implementing OAS itself.

We Swift developers hope to make this generator stable for reducing developing time.

@wing328

This comment has been minimized.

Copy link
Member

wing328 commented Oct 8, 2019

I see what you mean. I can only share my experience working on this project. You will always find users who want to use a particular HTTP library: Java client now supports 10+ HTTP libraries: feign, okhttp, Dart supports 2 or 3 via different generators. TS supports a lot via different generators.

Besides Alamofire, there's another one by IBM: https://github.com/IBM-Swift/SwiftyRequest

I'm not surprised if we are going to see a request from the user to support SwiftyRequest in the not too distant future.

My take is to support a swappable ApiClient class from day one and leave it up to the user to choose which HTTP library they want to use.

(and it's perfectly fine we disagree on different things)

@4brunu

This comment has been minimized.

Copy link
Contributor Author

4brunu commented Oct 9, 2019

@d-date I also prefer to reduce the number of dependencies, so it would be nice to rely on URLSession instead of Alamofire and I even considered creating a PR that removes Alamofire.

But on the other hand I think it would be nice to provide an optional integration with `RxSwift.

My concern with a big new Swift 5 generator is how long it will take, because we could introduce so many improvements, Result, Combine, URLSession, etc, that can take a long time.

This PR actually doesn't provide any value in itself, because it's the same as in Swift 4, but it will lower the barrier to anyone that wants to contribute with improvements to the Swift 5 generator.

What do you think of a more incremental approach?

By the way, I'm totally ok closing this PR 🙂

@wing328 wing328 removed this from the 4.2.0 milestone Oct 30, 2019
@4brunu 4brunu mentioned this pull request Dec 4, 2019
5 of 5 tasks complete
@Tovkal

This comment has been minimized.

Copy link

Tovkal commented Dec 4, 2019

I would merge this as is. My current use case is: I am using Swagger 2 until now, with Swift 5, and want to update to Swagger 3. So I just want to update the Swagger version while keeping the rest as it is.

I understand that there is room for improvement (decoupling from networking dependencies, adding other networking options) but I too believe this should be delivered incrementally.

@4brunu 4brunu mentioned this pull request Dec 12, 2019
5 of 5 tasks complete
@wing328 wing328 merged commit 5cc5fbe into OpenAPITools:master Jan 2, 2020
6 checks passed
6 checks passed
Shippable Run 13400 status is SUCCESS.
Details
ci/bitrise/4a2b10a819d12b67/pr Passed - openapi-generator
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr Build is passing
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wing328

This comment has been minimized.

Copy link
Member

wing328 commented Jan 2, 2020

IMPORTANT

This generator is relatively new and being actively developed. It's considered as beta/alpha and there will be breaking changes (new features, bug fixes, etc) to this generator without further notice.

jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Jan 5, 2020
* master: (275 commits)
  Initial CODEOWNERS (OpenAPITools#4924)
  [scala] Support for Set when array has uniqueItems=true (OpenAPITools#4926)
  remove nodejs server samples, scripts (OpenAPITools#4919)
  Added ability to work with `defaultHeaders` and fixed authentication for code generated by openapi-generator for typescript-node (OpenAPITools#4896)
  replace petstore_api with packageName (OpenAPITools#4921)
  remove base_object_spec.mustache from ruby client (OpenAPITools#4918)
  Add an link to Ada article (OpenAPITools#4920)
  avoid using hardcode prefix in example (OpenAPITools#4917)
  [dart-dio] Fix basepath (OpenAPITools#4911)
  [java][client] jackson update (OpenAPITools#4907)
  [Swift] Minor improvements to swift 5 generator (OpenAPITools#4910)
  [cpp-restbed] Added "out-of-the-box" functionality (OpenAPITools#4759)
  New generator swift5 (OpenAPITools#4086)
  [dart-dio] Adds support for multipart form data post body (OpenAPITools#4797)
  [go][client] fix when schema have multiple servers (OpenAPITools#4901)
  Unables CI tests of python-flask-python2 (OpenAPITools#4889)
  [C-libcurl] The JSON key name in request/response body should not be escaped even though it is a C key word. (OpenAPITools#4893)
  upgrade to JUnit 4.13 (OpenAPITools#4899)
  [r] Ignoring README.md in Rbuildignore (OpenAPITools#4898)
  update samples
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.