-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix CLLocationManager extension example #1208
Fix CLLocationManager extension example #1208
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.
Hi @phlippieb
} | ||
|
||
public func locationManager(_ manager: CLLocationManager, didUpdateLocations locations: [CLLocation]) { | ||
didUpdateLocationsSubject.onNext(locations) |
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.
This needs to forward call to _forwardToDelegate?.locationManager(manager, didUpdateLocations: locations)
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.
@kzaher Should it do that in addition to publishing to the subject, or instead of, or should it only publish to the subject if _forwardToDelegate
is nil?
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.
@phlippieb it should do both.
- publish to publish subject -> for observing
- forward it to forwarding delegate -> because of forwarding delegate definition
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.
@kzaher Roger that. Updated.
} | ||
|
||
public func locationManager(_ manager: CLLocationManager, didFailWithError error: Error) { | ||
didFailWithErrorSubject.onNext(error) |
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.
The same for this part.
Generated by 🚫 Danger |
Replaces PR 1027, which targeted master instead of develop.
Should fix #1136. This uses a similar method as RxScrollViewDelegateProxy.swift.