Skip to content
This repository has been archived by the owner on Dec 12, 2021. It is now read-only.

Migrate to Swift 4 #17

Merged
merged 1 commit into from
Oct 22, 2017
Merged

Migrate to Swift 4 #17

merged 1 commit into from
Oct 22, 2017

Conversation

jeffreylyg
Copy link

No description provided.

@jwhitley
Copy link
Contributor

FYI, .travis.yml is still using osx_image: xcode8.3, so its tests are bound to fail on an Xcode 9/Swift 4 branch.

@jwhitley
Copy link
Contributor

RxAutomaton.podspec needs to be updated to RxSwift 4.0, e.g.

s.dependency "RxSwift", "~> 4.0"

Copy link
Owner

@inamiy inamiy left a comment

Choose a reason for hiding this comment

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

@jeffreylyg
Hi, thank you for fixes!
Overall looks good to me 👍
Would you mind changing the parts I commented, and also the followings:

  • Change target branch to swift/4.0
  • Update .swift-version
  • Update Package.swift and Package.resolved
  • (If CI still fails) Comment-out pod lib lint in .travis.yml

@@ -38,7 +38,7 @@ class AutomatonViewController: UIViewController
{
return Observable<Int>.interval(interval, scheduler: MainScheduler.instance)
.take(count)
.scan(0) { $0.0 + 1 }
.scan(0) { ($0, $1).0 + 1 }
Copy link
Owner

Choose a reason for hiding this comment

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

{ x, _ in x + 1 } looks better.

@@ -137,9 +137,9 @@ public final class Automaton<State, Input>

// MARK: Private

private func _compose<A, B, C>(_ g: @escaping (B) -> C, _ f: @escaping (A) -> B) -> (A) -> C
private func _compose<A, B, C, D>(_ g: @escaping ((B) -> C), _ f: @escaping ((A, D) -> B)) -> ((A, D) -> C)
Copy link
Owner

Choose a reason for hiding this comment

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

Please change the use of type parameters in alphabetical order for readability, i.e.:

private func _compose<A, B, C, D>(_ g: @escaping ((C) -> D), _ f: @escaping ((A, B) -> C)) -> ((A, B) -> D)

@inamiy inamiy changed the base branch from swift/3.0 to swift/4.0 October 20, 2017 17:05
This was referenced Oct 20, 2017
Copy link
Owner

@inamiy inamiy left a comment

Choose a reason for hiding this comment

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

Thanks! Now it LGTM 👍

@inamiy inamiy merged commit 44f4999 into inamiy:swift/4.0 Oct 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants