fix(takeUntil): complete an observable when notifier completes

[DzmitryShylovich]

Closes #2160

From the original issue:

I think the correct behaviour would be to terminate when the notifier terminates, however at the very least the docs should be in line with the specs.

As for me it's much more intuitive.

Might be a breaking change though...

[coveralls]

Coverage increased (+0.0003%) to 97.687% when pulling 7eb7aa2 on DzmitryShylovich:gh/2160 into bc1e1e5 on ReactiveX:master.

[coveralls]

Coverage increased (+0.0003%) to 97.687% when pulling 7eb7aa2 on DzmitryShylovich:gh/2160 into bc1e1e5 on ReactiveX:master.

[jbaudanza]

I'm very much in favor of this change. If this isn't the way RxJs wants to go, it would be nice to have the docs updated to reflect this.

[benlesh]

I'm not sure what this breaking change buys us. Is there an example where one couldn't just compose a completed observable to necessarily next with concat?

source$.takeUntil(maybeEmpty$.concat(Observable.of(true)))

I'm going to close this for now, because there has been limited interest, and it's a breaking change from any existing version of Rx, with no discernible benefit.

[benlesh]

@DzmitryShylovich I really appreciate the effort, and I'm sorry to close this PR. The referenced issue this was meant to close was pretty clear the issue was with documentation and not with the functionality of the operator.

[jbaudanza]

@benlesh Fair enough. Would you accept a PR to fix discrepancy with the docs?

[DzmitryShylovich]

@jbaudanza see #2418

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.