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.

Sign up for GitHub

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

Jump to bottom

[Swift] Minor improvements to swift 5 generator #4910

Merged
merged 3 commits into from
Jan 3, 2020
Merged

[Swift] Minor improvements to swift 5 generator #4910

merged 3 commits into from
Jan 3, 2020

Conversation

wing328
Copy link
Member

@wing328 wing328 commented Jan 2, 2020 •
edited
Loading

cc @4brunu

  • rename to Swift5ClientCodegen to conform to other generators' naming
  • mark the generator as beta
  • minor code format change
  • update samples

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@wing328 wing328 added this to the 4.2.3 milestone Jan 2, 2020
@4brunu
Copy link
Contributor

4brunu commented Jan 2, 2020 •
edited
Loading

Can you please install or upgrade SwiftLint and generate the samples again?
brew install swiftlint && brew upgrade swiftlint

Should the section overview on the readme also be updated?

Otherwise it looks good 👍

@wing328
Copy link
Member Author

wing328 commented Jan 2, 2020 •
edited
Loading

Already installed swiftlint:

$ swiftlint help
Available commands:

   analyze         [Experimental] Run analysis rules
   autocorrect     Automatically correct warnings and errors
   generate-docs   Generates markdown documentation for all rules
   help            Display general or command-specific help
   lint            Print lint warnings and errors (default command)
   rules           Display the list of rules and their identifiers
   version         Display the current version of SwiftLint

But I may not be using the latest version. Checking...

@wing328
Copy link
Member Author

wing328 commented Jan 2, 2020

Should the section overview on the readme also be updated?

Done via dbf0733

@4brunu
Copy link
Contributor

4brunu commented Jan 2, 2020

That's weird, because it's not formatting the code.
Could you check which version do you have installed?
The last one is 0.38.0

@wing328
Copy link
Member Author

wing328 commented Jan 2, 2020

Updated swiftlint and samples via 4e903a8

@4brunu
Copy link
Contributor

4brunu commented Jan 2, 2020

Thanks 👍 For me this is ready to merge

@wing328 wing328 merged commit 7c31b7f into master Jan 3, 2020
@wing328 wing328 deleted the swift5-improve branch January 3, 2020 02:42
jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Jan 5, 2020
@jimschubert
Merge branch 'master' into feature-sets-meta
cafc61c 
* 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
  ...
@vegemanis
Copy link

@wing328 could you tell me why this is marked as Beta? Considering using this, but nervous about the beta status 🙂

@wing328
Copy link
Member Author

wing328 commented Mar 20, 2020

@vegemanis for new generators, we usually mark it as "beta" to start with and we'll consider upgrade it to "stable" when the generator stabilizes.

@vegemanis
Copy link

any existing concerns then or is this just procedural? Is there a list of requests for swift 5 that you're working off of?

@4brunu
Copy link
Contributor

4brunu commented Mar 20, 2020

Just procedural, and to give to community an opportunity to review it and improve it if necessary.
But I'm using it in production without issues.

@vegemanis
Copy link

Thanks @4brunu! Very helpful.

@Exey
Copy link

Exey commented Mar 30, 2020

4.3.0 still have warning in Model.swift at line 48

Cast from 'Dictionary<AnyHashable, Any>.Element' (aka '(key: AnyHashable, value: Any)') to unrelated type '(String, String)' always fails

Please fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Client: Swift Enhancement: General
Projects
None yet
Milestone
4.2.3
Development

Successfully merging this pull request may close these issues.
4 participants
@wing328 @4brunu @vegemanis @Exey