[swift5] Fix target SDKs for Combine option

[fl034]

Fixes #7728

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  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.
• File the PR against the correct branch: master, 5.1.x, 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[fl034]

@jgavris (2017/07) @ehyche (2017/08) @Edubits (2017/09) @jaz-ah (2017/09) @4brunu (2019/11)

[4brunu]

Hi,

First of all, thanks for taking the time to create this PR. 🙂

Could you please describe your use case on why to make this change?

I would like to discuss this PR.

The methods that use Combine are already marked as @available(OSX 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *).
So this should already protect agains users of iOS 12 and bellow.

Is this not enough?

It should be possible to choose more than one responseAs at the same time, for example Result and Combine.
In this case, the minimum supported version of iOS should be 9, although it still uses Combine.

The current setup of mark the Combine methods with @available(OSX 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) should allow the users of the library to choose Combine in iOS 13 and above, and Result on iOS 12 and bellow.

One thing that maybe we could do is to change the minimum iOS version to 13 when Combine is the only selected responseAs option?

@fl034 What do you think about this?

[fl034]

Hi @4brunu thanks for your discussion points.
Yeah the @available annotations aren't enough. When archiving it stops at the import Combine statement in every API-File.

I didn't know about the possibility to have multiple responseAs options selected.

So the best way would be to wrap import Combine with #canImport directives instead of my first approach, right?

[4brunu]

Hi @fl034,

Looks like a good solution to me.

Are you thinking in adding #canImport Combine in the import Combine and also the methods that use `Combine?

[4brunu]

Hi @fl034,

Thanks for making the changes that we talked about 👍

I think that we should also wrap the methods that use Combine with #canImport Combine.

What do you think about that?

[fl034]

Man you're fast! I was just writing about it. :D
I was thinking to replace the @available if #canImport, but this leads to errors.
And leaving it as it is and only wrapping the import statement also leads to errors.
So you're right, we need both. I will update it soon, again.

[4brunu]

Looks good, I just left a small comment. 👍
Thanks for your help!

[4brunu]

Could you please put #endif{{/useCombine}}{{#swiftUseApiNamespace}} in the same line to avoid generating an extra line in the generated code?

[fl034]

Jep :-)

[fl034]

@4brunu I noticed that in the latest snapshot version, using URLSession as library, I don't get callbacks from my requests. It works with Alamofire as library. Also it works in 5.0.0 stable.
Do you know if there's already an issue for that?

[4brunu]

No, I didn't knew about that.
Do you have a sample project to show the issue?
Because I'm using the snapshot version with URLSession and it's working.

[fl034]

@4brunu I opened an issue regarding my observation.
Since it's unrelated to this PR, you can merge this PR if you want :-).

[4brunu]

I don't have permissions to merge the PRs.
I only help reviewing them. 🙂
Let's just wait for the CI to finish.