Skip to content
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

Merged
merged 1 commit into from Oct 9, 2015

Conversation

artem-zinnatullin
Copy link
Contributor

Closes #3385.

testSubscriber.assertError(error);

verifyZeroInteractions(action);
}
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See doOnSuccessShouldNotSwallowExceptionThrownByAction()

@akarnokd akarnokd added this to the 1.0.x milestone Oct 8, 2015
* @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) {
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

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 notifications@github.com:

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added @Experimental

@akarnokd
Copy link
Member

akarnokd commented Oct 8, 2015

👍

@abersnaze
Copy link
Contributor

@artem-zinnatullin rebase please.

@artem-zinnatullin
Copy link
Contributor Author

@abersnaze rebased!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants