-
Notifications
You must be signed in to change notification settings - Fork 226
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
Implement coroutines interop module #374
Conversation
Tests are coming, just sticking up for early review for the implementation. Still not 100% sure I know what I'm doing with structured concurrency so want to get more eyes on this :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from my understanding of structured concurrency which is limited. The tests will give me a lot more confidence.
...terop/src/main/kotlin/com/uber/autodispose/coroutinesinterop/AutoDisposeCoroutinesInterop.kt
Show resolved
Hide resolved
These come transitively
ede28a3
to
d186f8f
Compare
Implemented the tests in d186f8f I'm still not totally sure about requiring a |
Went for it after digging through some coroutines sources. I think that's the right move. |
Super nice high order function with receiver API
Wrote something really nice building on top of the building blocks here. Now you can do this: scopeProvider.asCoroutineScope {
someObservable.autoDispose().subscribe()
} |
Pulled out the context APIs to #375 instead |
import kotlinx.coroutines.isActive | ||
import org.junit.Test | ||
|
||
class AutoDisposeCoroutinesInteropTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice tests!
This introduces a new interop artifact for interop between
Completable
/ScopeProvider
andCoroutineScope
. The idea is to offer this interop for general use, but also so anyone migrating from rx to coroutines (or the other way around!) can reuse existing machinery to ease migrations in the meantime.Resolves #371