search

xebia-functional / scala-days-ios

Official iOS app for Scala Days
http://www.scaladays.org
25 stars 8 forks source link

Migrate to Swift 4 #66

Closed juancazalla closed 6 years ago

juancazalla commented 6 years ago

Sorry, this review is huge 😅

juancazalla commented 6 years ago

One question about Crashlytics, can't we add the pod? @anamariamv @jdesiloniz

jdesiloniz commented 6 years ago

@juancazalla Sorry if I take my time with this one... First comment: as you mention I think we could use the Crashlytics pod. If I don't recall wrong, we couldn't use it back in the day, probably because they didn't have a podspec by then! If its pod doesn't work, maybe we could introduce it as a dependency but not including it in the PR (indicating in the README the steps to include the lib and how to compile for external users).

I hope to find more time later to keep with the review, but those were my first two cents on it :D.

jdesiloniz commented 6 years ago

@juancazalla I find most of the stuff alright. Could you check if you can get the Crashlytics/Fabric stuff out of the PR (ideally by using Pods)? I think with that sorted out we could think about merging this. Thanks!! :D

juancazalla commented 6 years ago

@jdesiloniz updated removing Crashlytics.framework and adding it to the podfile.