Skip to content

suggested update to SharedFlow.kt #3497

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

Closed

Conversation

Nodrex
Copy link

@Nodrex Nodrex commented Oct 20, 2022

Coroutines and Flows are amazing I love them and many many thanks for them 🤩🤩🤩🤩 You guys in JetBrains rock 🤘🤘🤘 🚀🚀🚀
I have a small suggestion:
According to this big discussion:
#2387 and a lot of confusion, including myself, default value of extraBufferCapacity should be 1 instead of 0 and I think this parameter also needs better explanation

Coroutines and Flows are amazing I love them and many many thanks for them 🤩🤩🤩🤩
You guys in JetBrains rock 🤘🤘🤘 🚀🚀🚀
I have a small suggestion:
According to this big discussion: 
Kotlin#2387 
and a lot of confusion, including myself, default value of `extraBufferCapacity` should be 1 instead of 0 and I think this parameter also needs better explanation
@qwwdfsad
Copy link
Collaborator

Thanks!

Unfortunately, we cannot accept this contribution -- it is a breaking change for stable API that will break almost every client of MutableSharedFlow on update.
The best way to affect the current behaviour is to initiate a discussion and maybe propose a draft solution/idea, as well as outline the downsides of the current behaviour and why the proposed solution outweighs benefits of the current solution

@qwwdfsad qwwdfsad closed this Oct 26, 2022
@Nodrex
Copy link
Author

Nodrex commented Oct 27, 2022

At least we can update docs, cause onBufferOverflow can be used to control backpressure on emitters (slowing them down). Is it not?

@qwwdfsad
Copy link
Collaborator

Yes, it probably makes sense

@Nodrex
Copy link
Author

Nodrex commented Oct 27, 2022

So should I send another pull request with just doc changes? @qwwdfsad

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 this pull request may close these issues.

2 participants