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

HandlerScheduler.scheduleDirect supports async option? #441

Closed
satoshun opened this issue Sep 4, 2018 · 4 comments · Fixed by #442
Closed

HandlerScheduler.scheduleDirect supports async option? #441

satoshun opened this issue Sep 4, 2018 · 4 comments · Fixed by #442

Comments

@satoshun
Copy link
Contributor

satoshun commented Sep 4, 2018

RxAndroid support async option v2.1.0. now, it support only in HandlerWorker.schedule.

I think, HandlerScheduler.scheduleDirect should be support async option.
Because, some RxJava operator use Scheduler.scheduleDirect method (ex, Maybe.observeOn)

(i prepared source code in 2.x...satoshun:2.x)

@ZacSweers
Copy link
Contributor

ZacSweers commented Sep 4, 2018

Hmm, that's a good shout. It'd be nicer if we could just use the hidden constructor of Handler to enable this at the handler level, but that implementation looks fine too. Looks like this affects all the observe/subscribe/dispose-"on" operators as well

@JakeWharton @adamp thoughts?

@JakeWharton
Copy link
Contributor

I'd rather do the simple thing. I was hoping the recent release would be the last. Maybe this one will stick...

@adamp
Copy link

adamp commented Sep 5, 2018

Consistency looks good to me.

@satoshun
Copy link
Contributor Author

satoshun commented Sep 5, 2018

thx reply!

Handler level async can use from API28 via Handler#createAsync. It makes the code more complicated.

I think that it is unnatural not to consider async option in HandlerScheduler.scheduleDirect.

I opened PR, If there is no problem please review it.

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 a pull request may close this issue.

4 participants