mysterioustrousers / MTMigration

Manages blocks of code that only need to run once on version updates in iOS apps.
BSD 2-Clause "Simplified" License
914 stars 50 forks source link

Added Swift version. #26

Closed hermanolsson closed 8 years ago

hermanolsson commented 8 years ago

We have been using MTMigration for quite some time, and during our deep dive into replacing most of our external libraries with Swift versions I figured that this little nifty class (the Swift version is a struct btw) should be converted to Swift instead of us looking for replacements or build our own.

I basically rewrote the class in Swift, with some minor tweaks to work with the language and as a framework.

Have a look and let me know if this is something of interest. I'd be more than happy to fix any issues you see with this version.

Big thanks from the Narrative Team.

yas375 commented 8 years ago

I'm wondering what are the benefits for this project being rewritten in Swift? You can use Swift and Objective-C together with no problem. You can make the integration nicer by marking parameters and return values nullable or not (https://developer.apple.com/swift/blog/?id=25). Seems easier then rewriting ;)

atomkirk commented 8 years ago

I'm in favor of Swift rewrites. With such awesome module support built into Swift and the annoyance of bridging headers, I'd like to see a near future where an entire project can go without a bridging header. So, I'm in favor. @pwightman as the original author, gonna leave it up to you.

hermanolsson commented 8 years ago

For this particular class, since it's so small, I don't see any huge advantages of using Swift, but in general I like to use Swift libraries because of the features it offers, like generics, type safety and some goodies like guard, enums and more. I did this because I figured it would be quite easy, and if I could contribute a bit, no one would be happier than me.

With this addition to the repo, users can choose whatever they like, and I had some fun (and also learned a bit about frameworks) while doing it.

Cheers!

yas375 commented 8 years ago

@hermanolsson :+1:

pwightman commented 8 years ago

Nice! Love this. Agreed on the Swift rewrite, it's small enough, might as well. Code style even looks :+1:, unique these days ;-)

The one niggle is that it appears the new tests aren't being run by travis? https://travis-ci.org/mysterioustrousers/MTMigration/builds/86695992

Probably just requires adding the test target to the travis.yml file. Are you familiar with travis @hermanolsson? Mind giving it a go? I might be able to fiddle with it later this week as well, I imagine it's a simple change.

hermanolsson commented 8 years ago

Sure, I'll have a look! I also noticed that I forgot to move over (and make swiftyfied) comments for each of the public methods.

hermanolsson commented 8 years ago

Well. I managed to get the tests working in the swift project, but now the objc one fails. There must be a gif or meme to represent this situation. =)

hermanolsson commented 8 years ago

I think the problem is that the project isn't even running on Xcode 7(.1). I even tried a fresh copy from master, and nothing works there either. The reason travis does not complain is because they still use Xcode 6 if no image is declared in the travis-file.

basememara commented 8 years ago

Great work on this!!

It's been 6 months and it doesn't look like the authors are too keen on replacing with Swift going forward. I wouldn't keep 2 versions parallel, just let obj-c die.

@hermanolsson I suggest starting a new repo/pod/carthage/spm and call it something like SwiftyMigration. You can always keep a back link to this project. This way, the Swift community can contribute freely and gain some speed. It's really a great and useful project, thanks to the original authors!

hermanolsson commented 8 years ago

@basememara I would be open to this, but it's really up to @pwightman to decide since the Swift implementation is really just a copy.

basememara commented 8 years ago

I believe the Swift version would be best with a fresh project instead of carrying the Obj-C baggage forward (target types, build settings, etc). Here is an example of how I would set up the fresh Swift project that would be forward compatible with Swift Package Manager and cross-platform for watchOS and tvOS too: http://goo.gl/qj1Qfj

Maybe @pwightman wants to start it and give you access or vice-versa and add him on it. I'd be glad to start the ground work too once we hear back from him.

pwightman commented 8 years ago

Sorry for the delay on this. Happy to merge and give relevant people commit access to make the Swift version compatible with swiftpm et. al (haven't used it yet myself, so not in a good position to implement it), if they would like. But if you think a fresh project is better, feel free to start one up, I don't mind. 👍 There's value in having both versions together, but I don't anticipate updating the Obj-C much in the long term, so if you plan on adding new features that you don't want to backport to Obj-C, then a separate project might be better.

basememara commented 8 years ago

Sweet, thx @pwightman :tada:

I forked it and created a proposed Swift project that is compatible with iOS, watchOS, and tvOS: https://github.com/ZamzamInc/MTMigration. Maybe you can create a new branch called swift and I'll create a pull request against it, then we can continue to evolve it from there?

Thanks for an amazing useful and awesome library btw 👍