-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
1.x: make Completable.subscribe() report isUnsubscribed consistently #3707
1.x: make Completable.subscribe() report isUnsubscribed consistently #3707
Conversation
@@ -1859,6 +1860,7 @@ public final Subscription subscribe(final Action0 onComplete) { | |||
subscribe(new CompletableSubscriber() { | |||
@Override | |||
public void onCompleted() { | |||
mad.unsubscribe(); |
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.
This one should be called after onComplete.call()
.
58826fa
to
a086b4f
Compare
a086b4f
to
00433f3
Compare
Updated. |
} | ||
mad.unsubscribe(); |
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.
Should be in finally
?
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.
Or just remove the above return?
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.
OnError calls it too, shouldn't call again after onError returns.
I see. 👍 |
1.x: make Completable.subscribe() report isUnsubscribed consistently
The empty and lambda-based
Completable.subscribe()
returns aSubscription
whoseisUnsubscribed
should be consistent with the rest of the reactive objects by returning true if the sequence terminated (not just when one truly cancelled it).