Skip to content

1.x: Add Completable.doOnCompleted and deprecate Completable.doOnComplete#3701

Merged
akarnokd merged 1 commit intoReactiveX:1.xfrom
zach-klippenstein:ObservableDoOnComplete1x
Feb 14, 2016
Merged

1.x: Add Completable.doOnCompleted and deprecate Completable.doOnComplete#3701
akarnokd merged 1 commit intoReactiveX:1.xfrom
zach-klippenstein:ObservableDoOnComplete1x

Conversation

@zach-klippenstein
Copy link

Closes #3700.

@akarnokd
Copy link
Member

Wait, what? I thought you wanted to add Completable.doOnCompleted(). I see no reason to change the established naming of Observable.doOnCompleted(). In fact, for consistency, I'd rather prefer adding Completable.doOnCompleted.

@zach-klippenstein
Copy link
Author

That makes sense. The only reason I did it this way is because on the 2.x branch both Observable and Completable use doOnComplete().

@akarnokd
Copy link
Member

Yes, those follow the reactive-streams convention and Completable started out as a 2.x addition. In the meantime, I'll fix that test failure.

@zach-klippenstein
Copy link
Author

Changed Completable instead, updated tests to match.

@akarnokd
Copy link
Member

👍

@zsxwing zsxwing changed the title 1.x: alias Observable.doOnCompleted to doOnComplete to match Completable and 2x 1.x: Add Completable.doOnCompleted and deprecate Completable.doOnComplete Feb 14, 2016
@zsxwing
Copy link
Member

zsxwing commented Feb 14, 2016

@zach-klippenstein Thanks! Just updated the title. 👍

akarnokd added a commit that referenced this pull request Feb 14, 2016
1.x: Add Completable.doOnCompleted and deprecate Completable.doOnComplete
@akarnokd akarnokd merged commit ee9956a into ReactiveX:1.x Feb 14, 2016
@stevegury
Copy link
Member

👍

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants