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

Introduce SharedFlow collect overload and override that return Nothing #2803

Merged
merged 8 commits into from Oct 20, 2021

Conversation

qwwdfsad
Copy link
Member

@qwwdfsad qwwdfsad commented Jul 1, 2021

* Override will ensure the proper implementation of the interface
* collect extension is added as the very basic lint helper

Fixes #2789
Fixes #2502

    * Override will ensure the proper implementation of the interface
    * collect extension is added as the very basic lint helper

Fixes #2789
Fixes #2502
@qwwdfsad qwwdfsad requested a review from elizarov July 1, 2021 13:16
@qwwdfsad
Copy link
Member Author

qwwdfsad commented Jul 1, 2021

I've decided not to add Nothing overload for each terminal extension as it seems to be too much of a hassle.
The basic lint for extension and core documentation should suffice

@LouisCAD
Copy link
Contributor

LouisCAD commented Aug 4, 2021

I've decided not to add Nothing overload for each terminal extension as it seems to be too much of a hassle.

Too much of a hassle to do, or from another standpoint like maintaining?

I'm asking to know if further PRs from the community to add them for terminal operators have a chance of getting merged.

@hfhbd
Copy link
Contributor

hfhbd commented Sep 28, 2021

Hey,
If I apply this patch and use a local object implementing SharedFlow:

interface Clock: SharedFlow<Int> { ... }
fun Clock() = object: Clock { }

the execution fails with KotlinNothingValueException before the second emit. Using an extra class class ClockImpl: Clock works without failing. Did you noticed this behavior too?

https://youtrack.jetbrains.com/issue/KT-48982

@qwwdfsad qwwdfsad requested review from dkhalanskyjb and removed request for elizarov October 13, 2021 09:34
Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good modulo some doc nits.

kotlinx-coroutines-core/common/src/flow/SharedFlow.kt Outdated Show resolved Hide resolved
kotlinx-coroutines-core/common/src/flow/SharedFlow.kt Outdated Show resolved Hide resolved
kotlinx-coroutines-core/common/src/flow/SharedFlow.kt Outdated Show resolved Hide resolved
qwwdfsad and others added 6 commits October 20, 2021 12:03
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
@qwwdfsad qwwdfsad merged commit e60685f into develop Oct 20, 2021
@qwwdfsad qwwdfsad deleted the shared-flow-collect branch October 20, 2021 09:04
@qwwdfsad
Copy link
Member Author

@LouisCAD to do :) Feel free to create a PR with additional overloads

yorickhenning pushed a commit to yorickhenning/kotlinx.coroutines that referenced this pull request Jan 28, 2022
Kotlin#2803)

* Introduce SharedFlow collect overload and override that return Nothing
* Override will ensure the proper implementation of the interface
* collect extension is added as the very basic lint helper

Fixes Kotlin#2789
Fixes Kotlin#2502

Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
pablobaxter pushed a commit to pablobaxter/kotlinx.coroutines that referenced this pull request Sep 14, 2022
Kotlin#2803)

* Introduce SharedFlow collect overload and override that return Nothing
* Override will ensure the proper implementation of the interface
* collect extension is added as the very basic lint helper

Fixes Kotlin#2789
Fixes Kotlin#2502

Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants