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

ObservableType extensions force observeOn calls #26

Closed
ggrell opened this issue Feb 3, 2017 · 3 comments
Closed

ObservableType extensions force observeOn calls #26

ggrell opened this issue Feb 3, 2017 · 3 comments
Milestone

Comments

@ggrell
Copy link

ggrell commented Feb 3, 2017

First time looking at the ModelMapper code (Thank You for implementing & sharing it!) I noticed something that is a problem:

The four map* functions contain observeOn calls in the chain. These really shouldn't be applied at this level because a) they cannot be overridden with my own scheduler, and b) should probably be left up to the subscriber (at the Moya call level) to properly observeOn and subscribeOn the correct schedulers.

I'm happy to submit a PR for this change. If not, I'll need to use a fork of the project because of this.

@sunshinejr
Copy link
Owner

Hey @ggrell, thanks for the issue. When I created this library I wanted to make sure no-one is mapping big objects using main thread. However I see where you coming from so after thinking about it for a little bit I think that I can agree to the change. Having in mind that Moya also added the queue as a parameter for request this is a great follow-up.

Unfortunately this means another major version bump, so I will do an issue for proper Swift 3.0 naming check so we can have everything in the next version altogether. Thanks again! 👊

@sunshinejr
Copy link
Owner

Damnit. Forgot about that one. Well, let's add it to 6.0.0 because Moya 10.0.0 is nearby anyways. 😄

@sunshinejr sunshinejr added this to the 6.0.0 milestone Sep 4, 2017
@sunshinejr
Copy link
Owner

It should be up in 6.0.0-beta.1. Closing this one now, but if anything pops out, don't hesitate to reopen it again!

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

No branches or pull requests

2 participants