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

2.x Single unsubscribeOn missing #5300

Closed
I60R opened this issue Apr 20, 2017 · 6 comments
Closed

2.x Single unsubscribeOn missing #5300

I60R opened this issue Apr 20, 2017 · 6 comments

Comments

@I60R
Copy link

I60R commented Apr 20, 2017

Also, in order to match 2.x naming convertions that operator should be named disposeOn

@akarnokd
Copy link
Member

Indeed that operator is missing from Single and available on the other types. Would you like to convert closest MaybeUnsubscribeOn over to it in a PR?

It is called unsubscribeOn for historical reasons and

  • to pair up with subscribeOn
  • to have the same name across the types instead of disposeOn and cancelOn.

unsubscribeOn is the name of (concept of) the operation whereas doOnCancel and doOnDispose represent the signal types and appropriate method names within the API and as such were adjusted.

@I60R
Copy link
Author

I60R commented Apr 20, 2017

Thanks for fast feedback and explanation.

About PR: yes, I can do it.
However, I'm not too experienced programmer and can't promise anything about code quality and time

@akarnokd
Copy link
Member

Oh, I understand. A certain code quality is expected here, plus RxJava 2.0.9 is scheduled to be released tomorrow. I'll do the PR then in a few hours.

@I60R
Copy link
Author

I60R commented Apr 20, 2017

Looks not too complicated. Can you give me 2-3 hours?

@akarnokd
Copy link
Member

I've done it. For exercise, see if you can do it yourself and compare it against #5302 to verify your approach.

@I60R
Copy link
Author

I60R commented Apr 20, 2017

Done it same way, but without testing (oh, it's hardest thing to prove that my code works as expected)

@I60R I60R closed this as completed Apr 21, 2017
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