search

google / promises

Promises is a modern framework that provides a synchronization construct for Swift and Objective-C.
Apache License 2.0
3.8k stars 293 forks source link

Updated to Swift 4.2 #70

Closed alexanderkhitev closed 5 years ago

googlebot commented 5 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers
alexanderkhitev commented 5 years ago

I signed it!

googlebot commented 5 years ago

CLAs look good, thanks!

alexanderkhitev commented 5 years ago

@shoumikhin Yes, I tested the launch of tests in Xcode Version 9.4.1, all tests are successful. I also looked in the settings for Xcode Version 9.4.1 to set Swift 4.1 as the default in this case. Please have a look at https://monosnap.com/file/z7QpilHT7HtHYeIEvW4cAeMsw1cPv7

ghost commented 5 years ago

Hey Alexander, thank you for the PR! Any specific reason why we should update the default Swift version from 4.0 to 4.2 at this time? Are you running into any issues in Xcode 10 with Swift 4.2? Thank you!

alexanderkhitev commented 5 years ago

Hey @temrich ! I ran the project on Xcode 10 and there it was indicated to update the project to Swift 4.2 in several pods. When I updated all pods and did prs, only the Promises framework was left, but this warning is easy to fix if I write config.build_settings ['SWIFT_VERSION'] = '4.2' or the path to my fork in the Podfile. I just decided to do PR to fix this warning without any additional action. Thanks for the framework, everything works fine.

ghost commented 5 years ago

Thank you for the quick reply! That makes sense.

Do you know if developers who still use 9.4.1 or lower (which I imagine is quite a few still) will see a warning if the targeted Swift version is updated to 4.2? Am wondering if we should hold off on this PR for a bit until more developers switch over to Xcode 10 and Swift 4.2?

alexanderkhitev commented 5 years ago

@temrich Among my friends, all the guys switched to Xcode 10 and Swift 4.2, of course I know that some frameworks (I can give an example), have not yet switched to Xcode 10 and Swift 4.2 and of course you can wait some more time.

wangjiejacques commented 5 years ago

@temrich for those who use Xcode 9.4.1, they can use the previous version, both Cocoapod and Cathage support specific version. I think it's OK to merge this pull request.

ssg commented 5 years ago

When is this going to be out?

shoumikhin commented 5 years ago

Just to clarify, what’s the main concern with not updating the Swift version now? Is that the Xcode suggestion that pops up every time you regenerate the project with CocoaPods? What other difficulties do you guys face with Promises not upgrading the Swift version?

ssg commented 5 years ago

I have the problem with the warning because the warning itself doesn't give out information on what needs to be migrated (so I can update the necessary packages), so I need to click on it and see "Promises" every time. I don't want to disable Swift conversion warnings either because we actually want to keep up with other pod updates.

soucolline commented 5 years ago

What's the main concern not to update ? Whomever did not upgrade to Xcode 10 / swift 4.2 can target 1.2.4, and the rest can get rid of this very annoying warning

shali3 commented 5 years ago

+1

ghost commented 5 years ago

Hey Alexander, thank you for the PR and sorry for the delayed reply. We are working on updating Travis to Xcode 10 and updating the Swift version from 4.0 to 4.2 in PR93. Would like to keep the Xcode and Swift version updates in the same PR.

Thank you!