Moving app to Swift
Closed this issue · 15 comments
Hey guys!
The app looks nice, it's very simple but looks gorgeous. I like such simple design ;) I found the repo via Open Sourcing Tropos, Our iOS Weather App. Gordon wrote about moving app to Swift. It means that new classes, etc. should be created in Swift or we can also play and rewrite existing classes in Swift?
Thanks!
I found your Tropos Trello board and there is some discussion about it, but probably no I see that nothing isn't confirmed yet.
I never used RAC earlier but heard about it and concept looks nice (would be great to learn it). I would like propose to move first classes that are not using RAC and when we're done with it we could switch to RAC 3.0 which works with Objective-C too so all the things should works correctly but we can do it slowly.
We have a card about recommending new functionality be written in Swift here but as you can see it's not complete :) There are some great resources in the comments there from WWDC 15 about obj-c swift interop.
We'd love to slowly move it over. What you said:
I would like propose to move first classes that are not using RAC and when we're done with it we could switch to RAC 3.0 which works with Objective-C too so all the things should works correctly but we can do it slowly.
Is pretty much exactly what I had in mind except that I hadn't considered moving classes not using RAC over first as well. That's a great idea.
The upgrading to RAC 3 will work in obj-c but will be a pretty large change because it has a significantly different API. I'm not sure how best to handle that. We'll probably have to do it all at once. It may be better to do that part sooner rather than later so that new Swift classes can use it as well.
Unfortunately RAC 3 still isn't out of beta so I'm not sure if we'd want to do that just yet. Perhaps we could start a branch to see what it would look like though.
For right now, I'd love more discussion on the best RAC migration strategy. It seems to be the main thing holding us back.
All that being said, we'd welcome PRs for converting existing classes to Swift. That seems fairly isolated and doable.
What is PRs? Don't know this shortcut :)
I saw the sources from WWDC two weeks ago, so it would be nice to use it in practice.
I started moving TRPrecipitation on my own, added first tests and I am replacing it with Swift version right now. I think I can start work on this, when my class is ready and project is configurated to use Swift it would be nice to convert classes by others too. I would like to avoid changing projects settings by everyone who want to start converting classes. What do you think? Can we wait for my pull request? If not, you could create Swift branch and I can create pull request with configuration so we can start work on it.
I also saw that project is using Specta and Expecta pods. Those are not compatible with Swift. Can we use XCTest instead? I played with Apple's XCTest and it is nice for testing Swift code and tests are even shorter ;)
I don't have any idea what to do with RAC3, I would rather discuss it with other because someone needs to implement it and changing RAC will blow the project because of different API.
Also, if we started moving classes for serious, do I need to create github issue for every class I want to change or I do it and then create pull request? Is there some ready solution to now doing the same work by few developers?
We should move to Quick and Nimble for Swift, not bare XCTest.
Okay @gfontenot thanks for suggestion.
Trying to integrate Quick and Nimble and have problems both for Swift 1.2 and Swift 2.0. Will tackle with it.
@gfontenot @jakecraige
Got another question. Should we start write in Swift 1.2 and after 2.0 is out convert the project? I think it would be best approach if you want to update app in App Store before 2.0 is out.
@tomkowz We should do Swift 1.2 for now. We're working on a new release right now and likely will have more before Swift 2 is out.
Related to RAC3, @hyperspacemark might have some insight on how bad upgrading to RAC3 would be.
"PR" is an abbreviation for "Pull request". "PRs" is "Pull requests".
No need to create an issue for each class you want to convert. If it's a large change and it going to take you a lot of time to do, it might be good to open up an issue first to confirm it's a change we're willing to make so that you don't do a lot of work and it gets turned down.
Okay great, I understand ;)
I would like to mention one more thing. In Swift we've got another requirement about class naming. In Objective-C prefix like here "TR" was required, in Swift it isn'not required anymore. Are you guys okay to drop TR prefix for classes that are written in Swift? Using Swift with Cocoa and Objective-C.
@tomkowz Please drop the prefix for Swift classes :)
@tomkowz Actually I'm not sure about how it works when you import Swift into obj-c regarding the prefix, mainly, do you need to keep it around?
Maybe @gfontenot or @hyperspacemark know for sure?
We can drop the prefix with Swift. Modules act as namespaces, so they aren't needed for interop.
Yup.
Hey, working on tests for WeatherUpdateCache. There are few mocks in tests for Objective-C class. Is there any framework we can use for Swift mocks and stubs? I won't expose methods from private to internal only because I cannot mock it with some library and have to subclass.
@tomkowz Unfortunately you can't really mock things the same way in swift as you do in obj-c. In Swift the approach is to use some form of dependency injection with a mock class that conforms to a protocol. So to mock things out the same way it may take bigger changes.
You can find a small example of this here with the FakeApiClient
And it's mentioned in this great talk from WWDC 2015 https://developer.apple.com/videos/wwdc/2015/?id=408
I thought so. Okay maybe protocol will help me.
Hey guys,
Just wanted to give you some updates to get you notified how to conversion process is going on.
I did a lot of work this week and I am little bit stopped by code review, but that's okay, it's code review, it have to take some time to get it done - learned few things this week so I found contribution and work with you awesome!
I have all work on my branch locally and I cherry-pick commits when doing pull requests and there are already all /Model, /ViewModel and /Formatters classes converted to Swift. Also, I converted UIColor+TRTemperatureColors to Swift so, ... we've got all tests Swift-only! 80+ Tests. This is good news because when 1 existing and 11+ future pull requests will be accepted we can drop Specta, Expecta and OCMock pods and write tests only in Swift. Isn't it great? : >
To keep converted classes in one place, here is the list:
Model:
- DailyForecast - In Review, #148
- Precipitation - Merged, #145
- Temperature - Merged, #141
- WeatherUpdate
- WeatherUpdateCache
ViewModel:
- DailyForecastViewModel
- WeatherViewModel
Formatters:
- PrecipitationChanceFormatter
- TemperatureFormatter
- DateFormatter
- BearingFormatter
- WindSpeedFormatter
- TemperatureComparisonFormatter
Categories:
- CATransition+FadeTransition
- CLLocation+RecentLocation
- NSBundle+BundleInfo
- NSDate+RelativeDate
- NSError+TRErrors - unused > removed
- NSNumber+TRRoundedNumber - unused > removed
- UIColor+TemperatureColors
- UIFont+AppFonts
What is still left to convert before RAC is updated to 3.0 is categories directory which should be converted to class extensions and then view classes, but for now have no idea how to properly test views. Do we want to test them? IMO this will be tough to test views.