freshOS / Then

:clapper: Tame async code with battle-tested promises
MIT License
993 stars 77 forks source link

Swift 5 support #52

Closed balecrim closed 5 years ago

balecrim commented 5 years ago

Sorry, I probably should've opened this issue before the PR. I'm mostly pasting the info from #51 here so that we can discuss these better. 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!

Thanks!

s4cha commented 5 years ago

@bernardowilson First of all thank you so much for taking the time to issue a PR, this is really appreciated :) I've had to relax the timings locally as well when testing with swift 5, it's not a big deal.

Concerning the breaking changes I need to check what's going on, we'll do that on #51.

PS: I'm currently trying to update to the latest Xcode at home with a terrible network connection might take some time ;)

arden commented 5 years ago

When will swift5 be supported ?

balecrim commented 5 years ago

Hi @s4cha, first of all, sorry for the radio silence lately, I haven't had as much time lately to help out. I'll look into your feedback on #51 and try and reproduce the differences between our environments and be sure to let you know ASAP what I find out.

v-ksenofontov commented 5 years ago

How about such a minimal revision?

extension Promise where T == Void {

    public convenience init(callback: @escaping (
        _ resolve: @escaping (() -> Void),
        _ reject: @escaping ((Error) -> Void)) -> Void) {
        self.init()
        setProgressCallBack { resolve, reject, _ in
            //swift 5 build fix:
            callback({
                resolve(())
            }, reject)
        }
    }

    public convenience init(callback2: @escaping (
        _ resolve: @escaping (() -> Void),
        _ reject: @escaping ((Error) -> Void),
        _ progress: @escaping ((Float) -> Void)) -> Void) {
        self.init()
        setProgressCallBack { resolve, reject, progress in
            //swift 5 build fix:
            callback2({
                resolve(())
            }, reject, progress)
        }
    }
}
s4cha commented 5 years ago

@vksenfontov eagle eye, indeed that's what I had on my side that enables migrating without all the changes highlighted on #51 here is mine for reference:

extension Promise where T == Void {

    public convenience init(callback: @escaping (
        _ resolve: @escaping (() -> Void),
        _ reject: @escaping ((Error) -> Void)) -> Void) {
        self.init()
        setProgressCallBack { resolve, reject, _ in
            let wrapped = { resolve(()) }
            callback(wrapped, reject)
        }
    }

    public convenience init(callback2: @escaping (
        _ resolve: @escaping (() -> Void),
        _ reject: @escaping ((Error) -> Void),
        _ progress: @escaping ((Float) -> Void)) -> Void) {
        self.init()
        setProgressCallBack { resolve, reject, progress in
            let wrapped = { resolve(()) }
            callback2(wrapped, reject, progress)
        }
    }
}
s4cha commented 5 years ago

@arden it is now :) @bernardowilson Congrats ! Thank you so much for you work 🥇

Release: https://github.com/freshOS/then/releases/tag/5.0.0 Pod: https://cocoapods.org/pods/thenPromise)

Closing this for now, don't hesitate to reopen if needed :)