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

[Swift 4] add compatibility for unwrapRequired config option #229

Merged
merged 2 commits into from
Jun 21, 2018

Conversation

joeboyscout04
Copy link
Contributor

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: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
    @jgavris, @ehyche, @Edubits, @jaz-ah, @d-date

Description of the PR

I added compatibility for unwrapRequired config option for Swift 4 to fix #228 .

@wing328 wing328 changed the title Issue 228 [Swift 4] add compatibility for unwrapRequired config option Jun 6, 2018
@wing328 wing328 added this to the 3.0.1 milestone Jun 6, 2018
@d-date
Copy link

d-date commented Jun 6, 2018

@joeboyscout04 Please let me know why you need this config in Swift 4 with your use case. More increased options, more hard to maintain source code safety.

@joeboyscout04
Copy link
Contributor Author

Hey there. This option was already existing in the config-help but when I tried it as both true and false, it didn’t do anything to change the generated code. When I looked at the source, the parameter was there for Swift4, but none of the implementation was present.

What I would like is a way to not have the required values unwrapped (unwrapRequired=false). Currently our API is changing a lot and if these parameter are unwrapped, the older apps will crash if one of the parameters is changed or removed. We like to be able to handle these nil values more safely.

@wing328 wing328 removed this from the 3.0.1 milestone Jun 11, 2018
@wing328 wing328 added this to the 3.0.3 milestone Jun 21, 2018
@wing328
Copy link
Member

wing328 commented Jun 21, 2018

The change looks good to me as the feature already presents in the Swift3 generator.

@wing328 wing328 merged commit 5b5fe3c into OpenAPITools:master Jun 21, 2018
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.

[Swift 4] unwrapRequired option doesn't do anything
3 participants