-
Notifications
You must be signed in to change notification settings - Fork 14
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
RxSwift / RxCocoa update to 5.1.1 / Deployment Target to 10.0 #175
base: master
Are you sure you want to change the base?
RxSwift / RxCocoa update to 5.1.1 / Deployment Target to 10.0 #175
Conversation
pod 'RxSwift', '~> 4.5' ==> ‘~> 5.1.1’ pod 'RxCocoa', '~> 4.5' ==> ‘~> 5.1.1’ # Added SwiftLint pod 'SwiftLint', '~> 0.40.0' pod 'RxTest', ‘’~> 4.5’ ==> ~> 5.1.1'
Orphaned Doc Comment Violation: A doc comment should be attached to a declaration. (orphaned_doc_comment)
…t 5.x) -- 'E' is deprecated: renamed to 'Element' -- 'Variable' is deprecated: Variable is deprecated. Please use `BehaviorRelay` as a replacement.
-- Cannot convert value of type 'Int' to expected argument type 'RxTimeInterval' (aka 'DispatchTimeInterval')
-- 'next' is deprecated: replaced by 'Recorded.next(_:_:)'
Updated IntrepidTesting deployment target from 8.0 to 12.0
Intrepid.podspec
Outdated
@@ -25,7 +25,7 @@ Pod::Spec.new do |s| | |||
s.source = { :git => "https://github.com/IntrepidPursuits/swift-wisdom.git", :tag => "#{s.version}" } | |||
s.exclude_files = "tests/**/*" | |||
s.platform = :ios | |||
s.ios.deployment_target = "10.0" | |||
s.ios.deployment_target = "12.0" |
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.
Have not yet done a full review pass, but just initially want to discuss why the deployment target is being increased. I know firsthand of some specific downstream apps that specifically require iOS 10 support (yes, I know that is generally overkill). So just want to think this through.
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.
Rolled back deployment_targets to 10.0
SwiftWisdom/Rx/Rx+Extensions.swift
Outdated
public func <- <T, O: ObservableType>(variable: Variable<T>, observable: O) -> Disposable where O.E == T { | ||
return observable.bind(to: variable) | ||
} | ||
//public func <- <T, O: ObservableType>(variable: T, observable: O) -> Disposable where O.Element == T { |
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.
Is there a reason to comment out this code rather than just deleting it?
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.
Just my paranoia - but the commented out code is deleted now
@@ -9,27 +9,27 @@ class OperatorTests: XCTestCase { | |||
func testVariableBinding() { | |||
let disposeBag = DisposeBag() | |||
let label = UILabel() | |||
let variable = Variable<String?>(nil) | |||
let variable = BehaviorRelay<String?>(value: 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.
Tiny thing, but I might consider renaming the bindings in these tests to relay
or behaviorRelay
instead of variable
.
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.
Not so tiny - renamed and removed duplicated tests!
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.
It looks good to me. Thanks for making these changes John!
@Nathaniel-Cox-Intrepid / @andrewdolce - how will you guys accept this PR? Will it be in a new branch rx_5.1.1 ? Or something different than that? |
@andrewdolce - are you OK with these changes? @andrewdolce / @Nathaniel-Cox-Intrepid - how do you want to land these changes? Into a new branch? Into a new release? I am just circling around to make sure this PR does not get lost in the mists of time! |
Got a warning from Apple that new app submissions will not be allowed to access UIWebView at all, and existing apps will not be able to submit new versions which access the UIWebView API starting in December 2020.
RxCocoa version 4.5 has a few extensions to UIWebView, and it seems like those extensions trigger the Apple check.
This pull request does several things:
** 'Variable' replaced with 'BehaviorRelay'
** 'E' replaced with 'Element'
** Errors around DispatchTimeInterval initialization no longer assuming seconds (i.e., "5" changed to ".seconds(5)")
** 'next' deprecated