-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
[swift5] Map file and binary to Data #9419
[swift5] Map file and binary to Data #9419
Conversation
@@ -115,7 +115,6 @@ public Swift5ClientCodegen() { | |||
"Bool", | |||
"Void", | |||
"String", | |||
"URL", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aymanbagabas why are you reming URL
from the list of primitive types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is duplicated in line 123
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right 👍
@@ -219,8 +218,8 @@ public Swift5ClientCodegen() { | |||
typeMapping.put("float", "Float"); | |||
typeMapping.put("number", "Double"); | |||
typeMapping.put("double", "Double"); | |||
typeMapping.put("file", "URL"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aymanbagabas could you please introduce a flag to control the typeMapping to be URL or Data, depending on this flag?
Because we need to give an option to the users to set the old behaviour if they need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is more of a "bug fix" than a new feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that this is a big change that can impact users.
From what we discuss in the issue, the file download is not working, bit the file upload is working as expected, and this can potentially break it.
So I think it's a good ideia to give an option to keep the old behaviour in case the users found any issue with this change.
samples/client/petstore/swift5/alamofireLibrary/docs/FakeAPI.md
Outdated
Show resolved
Hide resolved
2ef0df8
to
521776f
Compare
@aymanbagabas I was reviewing the open PRs and saw this one. |
Go ahead, I'm kinda busy nowadays and would really appreciate it |
521776f
to
3918f3d
Compare
.../openapi-generator/src/main/java/org/openapitools/codegen/languages/Swift5ClientCodegen.java
Outdated
Show resolved
Hide resolved
3918f3d
to
9ea1bd6
Compare
samples/client/petstore/swift5/alamofireLibrary/docs/FakeAPI.md
Outdated
Show resolved
Hide resolved
b81b8f4
to
4822a5a
Compare
@4brunu I think the docs CI is failing due to a missing generator not related to this PR. Travis CI couldn't pull the docker image and thus is failing. |
@aymanbagabas I think you are right, let's way for the other CI's to see if they pass |
* [swift5] Map file and binary to Data Fixes: OpenAPITools#9308 * Update docs
Fixes: #9308
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
,5.1.x
,6.0.x