-
Notifications
You must be signed in to change notification settings - Fork 1
swift 3.0 #1
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
swift 3.0 #1
Conversation
DispatchQueue.swift
Outdated
| public func async (callback: Void -> Void) { | ||
| dispatch_async(dispatch_queue) { callback() } | ||
| open func async (_ callback: @escaping (Void) -> Void) { | ||
| dispatch_queue.async { callback() } |
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.
minor thing, but can we do dispatch_queue.async(callback)?
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.
we can do dispatch_queue.async(execute: callback)
DispatchQueue.swift
Outdated
| open func sync (_ callback: @escaping (Void) -> Void) { | ||
| if isCurrent { callback(); return } // prevent deadlocks! | ||
| dispatch_sync(dispatch_queue) { callback() } | ||
| dispatch_queue.sync { callback() } |
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.
here too
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.
done
DispatchQueue.swift
Outdated
| public func async <T> (callback: T -> Void) -> T -> Void { | ||
| open func async <T> (_ callback: @escaping (T) -> Void) -> (T) -> Void { | ||
| return { [weak self] value in | ||
| if self == nil { return } |
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.
can we convert this to guard let or is it too overkill? :D
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.
actually can we do self?.async { callback(value) }?
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.
done, actually i removed too the force unwrap from bellow
DispatchQueue.swift
Outdated
| public func sync <T> (callback: T -> Void) -> T -> Void { | ||
| open func sync <T> (_ callback: @escaping (T) -> Void) -> (T) -> Void { | ||
| return { [weak self] value in | ||
| if self == nil { return } |
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.
here too
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.
done
DispatchQueue.swift
Outdated
| // MARK: Internal | ||
|
|
||
| init (_ queue: dispatch_queue_t) { | ||
| init (_ queue: Dispatch.DispatchQueue) { |
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.
do we need the Dispatch. prefix here and in the other usages?
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.
removed
DispatchQueue.swift
Outdated
|
|
||
| func remember () { | ||
| dispatch_queue_set_specific(dispatch_queue, &kCurrentQueue, getMutablePointer(self), nil) | ||
| guard let mutablePoiner = getMutablePointer(self) else { |
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.
small typo here Poiner
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.
done
| import Foundation | ||
| import UIKit | ||
|
|
||
| public typealias Timer = DispatchTimer |
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.
isn't this dangerous? Can it collide with NSTimer -> Timer?
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 can remove it all together but then we'll break the public API of the framework. i don't think it matters actually
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 removed this, Group and Dispatch typealiases
DispatchTimer.swift
Outdated
| dispatch_source_set_timer(timer, time, UInt64(delay_ns), UInt64(tolerance * CGFloat(NSEC_PER_SEC))) | ||
| dispatch_source_set_event_handler(timer) { [weak self] in let _ = self?.fire() } | ||
| dispatch_resume(timer) | ||
| let time = DispatchTime.now() + Double(Int64(delay_ns)) / Double(NSEC_PER_SEC) |
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 we don't need to divide by Double(NSEC_PER_SEC) if in the line before we don't multiply by CGFloat(NSEC_PER_SEC)
| let time = DispatchTime.now() + Double(Int64(delay_ns)) / Double(NSEC_PER_SEC) | ||
|
|
||
| let interval = DispatchTimeInterval.nanoseconds(Int(delay_ns)) | ||
| let leeway = DispatchTimeInterval.nanoseconds(Int(UInt64(tolerance * CGFloat(NSEC_PER_SEC)))) |
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 we can avoid double converting to UInt64 and Int
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.
are we sure that tolerance * CGFloat(NSEC_PER_SEC) will always be positive?
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.
you're converting to UInt64 anyway
DispatchQueue.swift
Outdated
| remember() | ||
| } | ||
|
|
||
| init (_ concurrent: Bool) { |
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.
can we remove the _ so that the concurrent argument is explicit?
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 removed all of them on this class
No description provided.