Skip to content
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

Update ReactiveSwift to 2.0 #86

Merged
merged 2 commits into from
Jul 25, 2017
Merged

Update ReactiveSwift to 2.0 #86

merged 2 commits into from
Jul 25, 2017

Conversation

ikesyo
Copy link
Collaborator

@ikesyo ikesyo commented Jul 24, 2017

No description provided.

@ikesyo ikesyo changed the title Update ReactiveSwift to 2.0 [WIP] Update ReactiveSwift to 2.0 Jul 24, 2017
@ikesyo ikesyo changed the title [WIP] Update ReactiveSwift to 2.0 Update ReactiveSwift to 2.0 Jul 24, 2017
@ikesyo
Copy link
Collaborator Author

ikesyo commented Jul 24, 2017

d09b749

Looks like this is due to ReactiveCocoa/ReactiveSwift#346.

// over `init(_ action: @escaping () -> Value)`.
let producer: SignalProducer<Any, Error> = SignalProducer { () -> Result<Any, Error> in
return JSONSerialization.deserializeJSON(data).mapError { Error.jsonDeserializationError($0.error) }
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let producer: SignalProducer<Any, Error> = SignalProducer {
    return JSONSerialization.deserializeJSON(data).mapError { Error.jsonDeserializationError($0.error) }
}

is not sufficient because Result<Any, Error> itself is treated as Any, so init(_ action: @escaping () -> Value) is still used.

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we should remove the () -> Value initializer—or rename it to be a static func. 😕

@andersio
Copy link

It is the @escaping () -> Value overload contributing to the ambiguity.

@ikesyo
Copy link
Collaborator Author

ikesyo commented Jul 24, 2017

Yes, I've fixed the description just before your comment. 😉

@mdiep mdiep merged commit fe46f82 into master Jul 25, 2017
@mdiep mdiep deleted the ras-2.0 branch July 25, 2017 12:31
@mdiep
Copy link
Owner

mdiep commented Jul 25, 2017

Thanks for this! 🤘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants