fix: don't treat notifier completion as a signal #5853
Conversation
cartant
changed the title
There was a problem hiding this comment.
Approved. This looks great.
The only thing I'd really like you to do @cartant is to clean up the commits, flattening out the "chore" type commits and breaking it into just the relevant fixes, such that each fix with a breaking change has a BREAKING CHANGE: ... comment in the commit message. Then just rebase and merge here on GitHub.
This way, the breaking changes will show up in the CHANGELOG when I generate it at release time. These are fixes, but they are liable to break someone I imagine.
BREAKING CHANGE: the observable returned by the audit operator's duration selector must emit a next notification to end the duration. Complete notifications no longer end the duration.
BREAKING CHANGE: the sample operator's notifier observable must emit a next notification to effect a sample. Complete notifications no longer effect a sample.
BREAKING CHANGE: the observable returned by the throttle operator's duration selector must emit a next notification to end the duration. Complete notifications no longer end the duration.
BREAKING CHANGE: the observable returned by the debounce operator's duration selector must emit a next notification to end the duration. Complete notifications no longer end the duration.
BREAKING CHANGE: the observable returned by the bufferWhen operator's closing selector must emit a next notification to close the buffer. Complete notifications no longer close the buffer.
|
@benlesh Done. I updated the docs, too. I'd missed those and they mentioned complete notifications. |
|
@benlesh I'm not sure about the change I made to P.S. The two commits that precede this were just some changes to the marble diagram alignments in |
|
@cartant |
BREAKING CHANGE: the observable returned by the bufferToggle operator's closing selector must emit a next notification to close the buffer. Complete notifications no longer close the buffer.
BREAKING CHANGE: the observable returned by the windowToggle operator's closing selector must emit a next notification to close the window. Complete notifications no longer close the window. Closes ReactiveX#5838
|
@benlesh Done. |
|
Nice. Thanks! |
Description:
This PR changes the behaviour of
audit,bufferToggle,bufferWhen,debounce,sample,throttleandwindowToggleso the that thecompletenotifications received from notifier/duration-selector observables are no longer treated as signals.BREAKING CHANGES: the behaviour of the above-mentioned operators has changed for empty notifiers/selectors.
Related issue (if exists): #5838