-
Notifications
You must be signed in to change notification settings - Fork 174
Swift 4.0 Support. #48
Swift 4.0 Support. #48
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one small comment but apart from that the changes look very good. Thank you!
SwiftMonkey/MonkeyXCTest.swift
Outdated
@@ -34,7 +34,7 @@ extension Monkey { | |||
let alert = application.alerts.element(boundBy: i) | |||
let buttons = alert.descendants(matching: .button) | |||
XCTAssertNotEqual(buttons.count, 0, "No buttons in alert") | |||
let index = UInt(self!.r.randomUInt32() % UInt32(buttons.count)) | |||
let index = Int(self!.r.randomUInt32() % UInt32(buttons.count)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use randomInt()
method here? The code would look like this:
let index = self!.r.randomInt(lessThan: buttons.count)
Credits to @mohamede1945 who used this in his PR (#47).
You're welcome, it's fixed 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
@DagAgren @DmitryBespalov I've checked the changes and I think they're fine. I we merge this branch and therefore migrate SwiftMonkey to Swift 4 we'll have to make sure that the next release of both Monkey and MonkeyPaws does not break any existing Swift 3 code. To ensure this I would suggest to release both libraries with a major version bump. This will indicate the breaking changes and hopefully will prevent people from automatically updating SwiftMonkey to this source-incompatible version. |
@@ -34,7 +34,7 @@ extension Monkey { | |||
let alert = application.alerts.element(boundBy: i) | |||
let buttons = alert.descendants(matching: .button) | |||
XCTAssertNotEqual(buttons.count, 0, "No buttons in alert") | |||
let index = UInt(self!.r.randomUInt32() % UInt32(buttons.count)) | |||
let index = self!.r.randomInt(lessThan: buttons.count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self!
might crash, please move it out to guard let, like this:
addAction(interval: interval) { [weak self] in
guard let this = self else { return }
...
let index = this.r.randomInt(...)
@@ -131,12 +131,12 @@ public class MonkeyPaws: NSObject, CALayerDelegate { | |||
let originalMethod = class_getInstanceMethod(UIApplication.self, originalSelector) | |||
let swizzledMethod = class_getInstanceMethod(UIApplication.self, swizzledSelector) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe guard let
these two variables to avoid force unwrapping below
I will merge this PR regardless of the suggested changes (consulted this with @DmitryBespalov). The mentioned code comments will be addressed in a separate PR. |
Upgraded all the things to Swift 4.0, not more than that.