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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Removed] RxMoyaProvider and ReactiveSwiftMoyaProvider #1320

Merged
merged 3 commits into from Sep 28, 2017

Conversation

Projects
None yet
3 participants
@SD10
Member

SD10 commented Sep 27, 2017

  • Removes the RxMoyaProvider and ReactiveSwiftMoyaProvider deprecated in v9.0.
  • Changes MoyaProviderRx -> MoyaProviderRxSpec 馃槈
  • Add Removed Changelog entry
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Sep 27, 2017

Codecov Report

Merging #1320 into 10.0.0-dev will increase coverage by 1.41%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           10.0.0-dev    #1320      +/-   ##
==============================================
+ Coverage       85.67%   87.09%   +1.41%     
==============================================
  Files              24       22       -2     
  Lines             768      736      -32     
==============================================
- Hits              658      641      -17     
+ Misses            110       95      -15

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 7c74420...6568c7b. Read the comment docs.

codecov-io commented Sep 27, 2017

Codecov Report

Merging #1320 into 10.0.0-dev will increase coverage by 1.41%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           10.0.0-dev    #1320      +/-   ##
==============================================
+ Coverage       85.67%   87.09%   +1.41%     
==============================================
  Files              24       22       -2     
  Lines             768      736      -32     
==============================================
- Hits              658      641      -17     
+ Misses            110       95      -15

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 7c74420...6568c7b. Read the comment docs.

@sunshinejr sunshinejr referenced this pull request Sep 27, 2017

Closed

Release 10.0 #1253

9 of 11 tasks complete
@@ -6,7 +6,7 @@ import OHHTTPStubs
@testable import Moya
@testable import RxMoya
final class MoyaProviderRx: QuickSpec {
final class MoyaProviderRxSpec: QuickSpec {

This comment has been minimized.

@sunshinejr

sunshinejr Sep 27, 2017

Member

馃憣

@sunshinejr

sunshinejr Sep 27, 2017

Member

馃憣

@sunshinejr

This comment has been minimized.

Show comment
Hide comment
@sunshinejr

sunshinejr Sep 27, 2017

Member

Thanks for the work, @SD10! One thing that I didn't say before is that with removal of reactive providers we don't need additional abstraction over reactive methods (we needed these as we shared code (e.g. rx.request and RxMoyaProvider<>().request). Would you like to remove the unnecessary abstraction as well? ( e.g. here and in ReactiveSwift extensions as well)

+

could you fix these 2 linting issues as well? 馃槈

Member

sunshinejr commented Sep 27, 2017

Thanks for the work, @SD10! One thing that I didn't say before is that with removal of reactive providers we don't need additional abstraction over reactive methods (we needed these as we shared code (e.g. rx.request and RxMoyaProvider<>().request). Would you like to remove the unnecessary abstraction as well? ( e.g. here and in ReactiveSwift extensions as well)

+

could you fix these 2 linting issues as well? 馃槈

@SD10

This comment has been minimized.

Show comment
Hide comment
@SD10

SD10 Sep 28, 2017

Member

@sunshinejr I'm a little confused about the refactoring that needs to be done to remove the two abstractions. It requires I prefix all MoyaProvider methods with .rx.base & .reactive.base then.

I tried moving the methods back into extension Reactive where Base: MoyaProviderType but have problems regarding capturing self on a non-class constrained type.

Feel free to take over if you'd like

Member

SD10 commented Sep 28, 2017

@sunshinejr I'm a little confused about the refactoring that needs to be done to remove the two abstractions. It requires I prefix all MoyaProvider methods with .rx.base & .reactive.base then.

I tried moving the methods back into extension Reactive where Base: MoyaProviderType but have problems regarding capturing self on a non-class constrained type.

Feel free to take over if you'd like

@sunshinejr

This comment has been minimized.

Show comment
Hide comment
@sunshinejr

sunshinejr Sep 28, 2017

Member

@SD10 oh, let's just fix these linting issues then and we can do the refactor in another PR, no worries!

Member

sunshinejr commented Sep 28, 2017

@SD10 oh, let's just fix these linting issues then and we can do the refactor in another PR, no worries!

@sunshinejr

Thanks, @SD10!

@sunshinejr sunshinejr merged commit 059979c into 10.0.0-dev Sep 28, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@sunshinejr sunshinejr deleted the remove/reactive-provider-subclasses branch Sep 28, 2017

sunshinejr added a commit that referenced this pull request Oct 2, 2017

sunshinejr added a commit that referenced this pull request Oct 3, 2017

AndrewSB added a commit that referenced this pull request Oct 3, 2017

sunshinejr added a commit that referenced this pull request Oct 8, 2017

sunshinejr added a commit that referenced this pull request Oct 8, 2017

sunshinejr added a commit that referenced this pull request Oct 8, 2017

sunshinejr added a commit that referenced this pull request Oct 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment