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

Bump ReactiveSwift to 5.0 and Result to 4.1 #1817

Merged
merged 2 commits into from Mar 19, 2019

Conversation

Projects
None yet
3 participants
@larryonoff
Copy link
Contributor

larryonoff commented Mar 18, 2019

Bump ReactiveSwift to 5.0 and Result to 4.1

@larryonoff larryonoff force-pushed the larryonoff:bump-reactiveswift branch 2 times, most recently from fbbbc3c to e9cb4c8 Mar 18, 2019

@sunshinejr

This comment has been minimized.

Copy link
Member

sunshinejr commented Mar 18, 2019

Hey @larryonoff, thanks for the PR! Was there something specific that happened in Result 4.1 that we don't want to support version 4.0?

@larryonoff

This comment has been minimized.

Copy link
Contributor Author

larryonoff commented Mar 18, 2019

@sunshinejr ReactiveSwift 5.0 depends on Result 4.1.

ReactiveSwift 5.0 and Result 4.1 both add Swift 5.0 and Xcode 10.2.

@sunshinejr

This comment has been minimized.

Copy link
Member

sunshinejr commented Mar 18, 2019

@larryonoff makes sense, thanks! Would you please update SPM lock (probably swift package update if I'm not mistaken)?

+ I see that the project doesn't want to build tests due to Alamofire update... Interestingly enough it seems like they've made a breaking change without a major update 🤔

@larryonoff larryonoff force-pushed the larryonoff:bump-reactiveswift branch from e9cb4c8 to 65ab233 Mar 18, 2019

@larryonoff

This comment has been minimized.

Copy link
Contributor Author

larryonoff commented Mar 18, 2019

@larryonoff

This comment has been minimized.

Copy link
Contributor Author

larryonoff commented Mar 18, 2019

@sunshinejr

I see that the project doesn't want to build tests due to Alamofire update... Interestingly enough it seems like they've made a breaking change without a major update 🤔

I'm not sure, but it was without update.

@sunshinejr

This comment has been minimized.

Copy link
Member

sunshinejr commented Mar 19, 2019

@larryonoff when you look at the Cartfile.resolved (we use Carthage dependencies for building/testing), you can see that the Alamofire has bumped version to 4.8.1. Then, when we look at the build error in tests:

  /Users/distiller/Moya/Moya/Tests/EndpointClosureSpec.swift:65:19: method does not override any method from its superclass

    override func upload(multipartFormData: @escaping (Alamofire.MultipartFormData) -> Void, usingThreshold encodingMemoryThreshold: UInt64, with urlRequest: URLRequestConvertible, encodingCompletion: ((SessionManager.MultipartFormDataEncodingResult) -> Void)?) {

it seems like Alamofire updated this method in their minor/patch release. If you have time to fix this one it would be great! If not, I could try to look around and help you out so no worries 👍

@larryonoff

This comment has been minimized.

Copy link
Contributor Author

larryonoff commented Mar 19, 2019

@sunshinejr I have fixed it in the current branch. Please review

@MoyaBot

This comment has been minimized.

Copy link

MoyaBot commented Mar 19, 2019

3 Messages
📖 iOS: Executed 278 tests, with 0 failures (0 unexpected) in 13.325 (13.483) seconds
📖 tvOS: Executed 278 tests, with 0 failures (0 unexpected) in 13.065 (13.207) seconds
📖 macOS: Executed 278 tests, with 0 failures (0 unexpected) in 15.028 (15.138) seconds

Generated by 🚫 Danger

@sunshinejr
Copy link
Member

sunshinejr left a comment

Awesome thanks @larryonoff!

@sunshinejr sunshinejr merged commit 42d09ef into Moya:development Mar 19, 2019

2 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: carthage_without_swiftlint_integration Your tests passed on CircleCI!
Details

@larryonoff larryonoff deleted the larryonoff:bump-reactiveswift branch Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.