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

Xcode 8.3 support #123

Merged
merged 10 commits into from
Mar 30, 2017
Merged

Xcode 8.3 support #123

merged 10 commits into from
Mar 30, 2017

Conversation

mbrandonw
Copy link
Contributor

@mbrandonw mbrandonw commented Mar 28, 2017

Adds support for Xcode 8.3 and Swift 3.1. Same sitch as kickstarter/ios-ksapi#21, kickstarter/Kickstarter-Prelude#70 and kickstarter/Kickstarter-ReactiveExtensions#61.

One cool thing about Xcode 8.3... playgrounds work again! We can get back to our old Xcode 7.x flow of using playgrounds for UI dev.

Todo

  • circleci xcode 8.3 support
  • figure out static func initialize stuff

@@ -30,17 +30,17 @@ public final class ReferralChartView: UIView {

guard let context = UIGraphicsGetCurrentContext() else { return }

let internalPercentageAngle = CGFloat(-M_PI_2) + self.internalPercentage * CGFloat(2.0 * M_PI)
let internalPercentageAngle = CGFloat(-.pi / 2.0) + self.internalPercentage * CGFloat(2.0 * .pi)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a fun one! pi has been moved to a static var on Double, and the type-inference makes it really niiiiice

Copy link
Contributor

Choose a reason for hiding this comment

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

ah nice!

@@ -118,7 +118,7 @@ public struct AppEnvironment {
locale: Locale = AppEnvironment.current.locale,
mainBundle: NSBundleType = AppEnvironment.current.mainBundle,
reachability: SignalProducer<Reachability, NoError> = AppEnvironment.current.reachability,
scheduler: DateSchedulerProtocol = AppEnvironment.current.scheduler,
scheduler: DateScheduler = AppEnvironment.current.scheduler,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a rename from ReactiveSwift 1.1.0

@@ -25,6 +25,8 @@ private func swizzle(_ v: UIView.Type) {
}

extension UIView {
// FIXME: "Method 'initialize' defines Objective-C class method, which is not guaranteedto be invoked by
// by Swift and will be disallowed in future versions.
open override class func initialize() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gotta figure this one out before merging :/ still thinking about possible solutions

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps just function calls from AppDelegate didFinishLaunchingWithOptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 going that route

@@ -41,7 +41,7 @@ DeprecatedWebViewModelOutputs {
// Hide loading if the web view fails with a non-102 error code (102 is the interrupted error that
// occurs anytime we cancel a request).
self.webViewDidFailErrorProperty.signal
.filter { ($0 as? NSError)?.code != .some(102) }
.filter { ($0 as NSError?)?.code != .some(102) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't know why this works, but whatever it does. feel there is some error error/nserror bridging going on, and not sure i care enough to know why...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think it's because Error casting to NSError will always succeed? so the ? just moves from the as to NSError because it's specifically casting it as Optional<NSError>.

This could also be:

self.webViewDidFailErrorProperty.signal
  .skipNil()
  .map { $0 as NSError }
  .filter { $0.code != .some(102) }
  .mapConst((true, true))
)
.skipRepeats(==)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes that makes sense, forgot that $0 was optional at that moment

Copy link
Contributor

@justinswart justinswart left a comment

Choose a reason for hiding this comment

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

Nice thanks for doing this! I hear SourceKit is way more stable in 8.3 👍

@@ -41,7 +41,7 @@ DeprecatedWebViewModelOutputs {
// Hide loading if the web view fails with a non-102 error code (102 is the interrupted error that
// occurs anytime we cancel a request).
self.webViewDidFailErrorProperty.signal
.filter { ($0 as? NSError)?.code != .some(102) }
.filter { ($0 as NSError?)?.code != .some(102) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think it's because Error casting to NSError will always succeed? so the ? just moves from the as to NSError because it's specifically casting it as Optional<NSError>.

This could also be:

self.webViewDidFailErrorProperty.signal
  .skipNil()
  .map { $0 as NSError }
  .filter { $0.code != .some(102) }
  .mapConst((true, true))
)
.skipRepeats(==)

@@ -125,7 +125,7 @@ public final class LoginToutViewModel: LoginToutViewModelType, LoginToutViewMode
.mapConst(AlertError.facebookTokenFail)

let facebookLoginAttemptFailAlert = self.facebookLoginFailProperty.signal
.map { $0 as? NSError }
.map { $0 as NSError? }
Copy link
Contributor

Choose a reason for hiding this comment

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

Or if we wanted, but same-same:

let facebookLoginAttemptFailAlert = self.facebookLoginFailProperty.signal
  .skipNil()
  .map { $0 as NSError }
  .map(AlertError.facebookLoginAttemptFail)

@@ -25,6 +25,8 @@ private func swizzle(_ v: UIView.Type) {
}

extension UIView {
// FIXME: "Method 'initialize' defines Objective-C class method, which is not guaranteedto be invoked by
// by Swift and will be disallowed in future versions.
open override class func initialize() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps just function calls from AppDelegate didFinishLaunchingWithOptions?

@@ -30,6 +30,9 @@ internal final class AppDelegate: UIResponder, UIApplicationDelegate {
_ application: UIApplication,
didFinishLaunchingWithOptions launchOptions: [UIApplicationLaunchOptionsKey: Any]?) -> Bool {

UIView.doBadSwizzleStuff()
UIViewController.doBadSwizzleStuff()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

welp here's where we're at. doing our bad swizzle stuff first thing when the app launches.

at least we are being honest now and not hiding deep somewhere else!

@@ -28,6 +28,9 @@ internal class TestCase: FBSnapshotTestCase {
override func setUp() {
super.setUp()

UIView.doBadSwizzleStuff()
UIViewController.doBadSwizzleStuff()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh also we have to do it tests for screenshots :P

@mbrandonw mbrandonw merged commit d839fc5 into master Mar 30, 2017
@mbrandonw mbrandonw deleted the xcode-8.3 branch March 30, 2017 21:15
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

2 participants