-
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
Add Single.doOnSuccess() #3417
Add Single.doOnSuccess() #3417
Conversation
bfb1ee7
to
7d73e2c
Compare
testSubscriber.assertError(error); | ||
|
||
verifyZeroInteractions(action); | ||
} |
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.
Could you also test what happens if the action
callback throws?
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.
See doOnSuccessShouldNotSwallowExceptionThrownByAction()
* @return the source {@link Single} with the side-effecting behavior applied | ||
* @see <a href="http://reactivex.io/documentation/operators/do.html">ReactiveX operators documentation: Do</a> | ||
*/ | ||
public final Single<T> doOnSuccess(final Action1<? super T> onSuccess) { |
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 should start out as experimental unless the RxJava contributors want to fast-track this.
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.
I didn't add @Experimental
here and in the other PRs because whole class Single
is marked with @Experimental
. Do you think we should mark these basic operators too?
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.
Yep, this is a bit of a conflict because #3401 gets merged first, this would also become beta without any votes.
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.
Okay, no problem. I'll mark it as @experimental.
чт, 8 Окт 2015, 12:44, David Karnok [email protected]:
In src/main/java/rx/Single.java
#3417 (comment):@@ -1789,4 +1790,36 @@ public void onNext(T t) {
return zip(this, other, zipFunction);
}
- /**
\* Modifies the source {@link Single} so that it invokes an action when it calls {@code onSuccess}.
\* <p>
\* <img width="640" height="310" src="https://raw.github.com/wiki/ReactiveX/RxJava/images/rx-operators/doOnNext.png" alt="">
\* <dl>
\* <dt><b>Scheduler:</b></dt>
\* <dd>{@code doOnSuccess} does not operate by default on a particular {@link Scheduler}.</dd>
\* </dl>
*
\* @param onSuccess
\* the action to invoke when the source {@link Single} calls {@code onSuccess}
\* @return the source {@link Single} with the side-effecting behavior applied
\* @see <a href="http://reactivex.io/documentation/operators/do.html">ReactiveX operators documentation: Do</a>
*/
- public final Single doOnSuccess(final Action1<? super T> onSuccess) {
Yep, this is a bit of a conflict because #3401
#3401 gets merged first, this
would also become beta without any votes.—
Reply to this email directly or view it on GitHub
https://github.com/ReactiveX/RxJava/pull/3417/files#r41494676.
@artem_zin
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.
Added @Experimental
7d73e2c
to
4249c6a
Compare
👍 |
@artem-zinnatullin rebase please. |
4249c6a
to
0c0a18f
Compare
@abersnaze rebased! |
Closes #3385.