-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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] Add option parameter to select response queue #4591
Conversation
This is nice, because it maintains backwards compatibility and it's configurable. |
Run ./bin/swift4-all.sh then remove unrelated diff.
@@ -11,6 +11,7 @@ open class {{projectName}}API { | |||
public static var credential: URLCredential? | |||
public static var customHeaders: [String:String] = [:] | |||
public static var requestBuilderFactory: RequestBuilderFactory = AlamofireRequestBuilderFactory() | |||
public static var queue: DispatchQueue = .main |
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.
@syatyo what do you think of giving this a more specific name?
Something like APIResponseQueue
or something like that.
It's just to avoid confusion in case more configurable queue's are added in the future.
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.
Variable names in swift start with lowercase, so maybe something like apiResponseQueue
?
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.
It seems better! I will try it.
Rename queue to apiResponseQueue, because apiResponseQueue is more clear for explaining the context.
Thanks for the changes, for me it looks good 🙂 |
@syatyo thanks for the PR. Can you please resolve the merge conflicts when you've time? |
@@ -11,6 +11,7 @@ open class {{projectName}}API { | |||
public static var credential: URLCredential? | |||
public static var customHeaders: [String:String] = [:] | |||
public static var requestBuilderFactory: RequestBuilderFactory = AlamofireRequestBuilderFactory() | |||
public static var apiResponseQueue: DispatchQueue = .main |
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.
@syatyo Since now the Swift generator support the visibility modifier, this line should become
{{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} static var apiResponseQueue: DispatchQueue = .main
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've tried to fix it via 404288d
@wing328 Thanks for resolving conflics! |
@syatyo thanks for the PR, which has been included in the v4.2.2 release: https://twitter.com/oas_generator/status/1201432648544972800 |
Description of the PR
Proposed feature for [REQ] [Swift] Select response queue
@jgavris @ehyche @Edubits @jaz-ah @d-date
close #4590
PR checklist
./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).master
,4.3.x
,5.0.x
. Default:master
.