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

Adding tentative Swift 5 support, relaxing test timing #51

Merged
merged 1 commit into from Apr 24, 2019

Conversation

balecrim
Copy link

@balecrim balecrim commented Apr 3, 2019

Hi, I've begun working on integrating the changes necessary to allow compilation of the project on Xcode 10.2 using its Swift 5 toolchain.

All tests are currently passing, but I've had to relax the timings on some of them from 0.3 to 0.5 seconds in order to run successfully on simulators. Is that a problem?

Some changes, such as the ones made to Promise will be breaking the current API, as the compiler can't (any longer?) disambiguate the types on its own. I'm open to ideas on these solutions!

Added initial Swift 5 support by disambiguating void promises' init closures. Includes adaptations to test suites.

This commit relaxes some tests' timing in order to allow them to pass on the iOS simulator. Both

RegisterThenTests.testRegisterThenPromiseFuncPointerCalledWithMultipleRegisterThenBlocks()
ThenTests.testThen()

have had their timeouts relaxed to 0.5 seconds in order to consistently run on my simulator and real devices (iPad Pro 2018 and iPhone XR).

Signed-off-by: Bernardo Alecrim <bernardo@ltmdev.com>
@balecrim balecrim mentioned this pull request Apr 4, 2019
Copy link
Member

@s4cha s4cha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried on my machine and somehow some files you modified work fine untouched on my side. ( it's on ad2dfd7 branch swift5) if you want to give it a look so we can discuss. It looks like we could upgrade without breaking any public api 👍

@@ -27,8 +27,9 @@ extension Promise {

extension Promises {
public static func delay(_ time: TimeInterval) -> Promise<Void> {
return Promise { (resolve: @escaping (() -> Void), _: @escaping ((Error) -> Void)) in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part complies fine without changing anything on my end, any reason why this changed?

return Promise { resolve, _ in resolve(value) }
}
}

extension Promise where T == Void {
public class func resolve() -> Promise<Void> {
return Promise { resolve, _ in resolve() }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part compiles without changes on my end as well.

@@ -11,7 +11,7 @@ import Foundation
extension Promise where T == Void {

public convenience init(callback: @escaping (
_ resolve: @escaping (() -> Void),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two seem to compile untouched on my side as well

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for me after upgrading an existing 4.2 project to swift 5 inside xcode 10.2.1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, for me neither. I'll look into recloning the project and seeing if I can reproduce why it's building on @s4cha's machine.

@@ -115,25 +115,35 @@ func waitTime(_ time: Double, callback: @escaping () -> Void) {
}

func upload() -> Promise<Void> {
return Promise { (resolve: @escaping (() -> Void), _: @escaping ((Error) -> Void), progress) in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is fine untouched on my end

@s4cha s4cha merged commit 6b32bca into freshOS:master Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants