Add Single.finallyDo()

[artem-zinnatullin]

@akarnokd I've found a problem with null action, I'll create separate issue about this soon.

[akarnokd]

Could you verify what happens ig the callback throws?

One should check null in assembly time (i.e. when calling Single.finallyDo.

[artem-zinnatullin]

@akarnokd unfortunately very bad thing happens — not only this exception will be swallowed, but also finallyAction executed twice… I'm investigating this and will create issue soon.

[akarnokd]

👍

[abersnaze]

um... I just noticed that we doOnTerminate and finallyDo on Observable.

Could we make this look like doOn*()?

[JakeWharton]

doOnTerminate happens before subscriber notification. finallyDo happens after.

[artem-zinnatullin]

Rebased

[akarnokd]

It can be called doOnAfterComplete() but since finallyDo is established, I'm not sure this rename can happen for 1.0.x.

[stealthcode]

I'd rather it were called doAfterComplete. finallyDo only exists for the Observable and it doesnt follow the do* convention. We could choose to have 2 conventions (naming Single's variant similarly) or deprecate Observable#finallyDo and create Observable#doAfterComplete. I vote for the latter naming them both doAfterComplete.

[JakeWharton]

I think it would have to be doAfterTermination since it runs on both
complete and error.

On Sat, Oct 17, 2015, 2:45 PM Aaron Tull notifications@github.com wrote:

I'd rather it were called doAfterComplete. finallyDo only exists for the
Observable and it doesnt follow the do* convention. We could choose to have
2 conventions (naming Single's variant similarly) or deprecate
Observable#finallyDo and create Observable#doAfterComplete. I vote for the
latter naming them both doAfterComplete.

—
Reply to this email directly or view it on GitHub
#3434 (comment).

[stealthcode]

Does anyone have any objection to creating Observable.doAfterTermination as an alias for finallyDo and deprecating the original finallyDo? If that's acceptable then this PR should name it Single.doAfterTermination.

[akarnokd]

👍 doAfterTermination

[davidmoten]

A bit of consistency in tenses would be nice though.

We have

doOnCompleted
doOnTerminate
doOnSubscribe
doOnRequest
...

doOnCompleted is the odd one out but may as well live with that. To be consistent call the method in question doAfterTerminate or doAfterTerminated?

[stealthcode]

I think doAfterTerminate sounds best to me. It's consistent with the
onComplete of 2.x

On Sat, Dec 5, 2015, 01:53 Dave Moten notifications@github.com wrote:

A bit of consistency in tenses would be nice though.

We have

doOnCompleted
doOnTerminate
doOnSubscribe
doOnRequest
...

doOnCompleted is the odd one out but may as well live with that. To be
consistent call the method in question doAfterTerminate or
doAfterTerminated?

—
Reply to this email directly or view it on GitHub
#3434 (comment).

[artem-zinnatullin]

Renamed to doAfterTerminate & rebased, PTAL.

[stealthcode]

Does anyone have any objection to creating Observable.doAfterTermination as an alias for finallyDo and deprecating the original finallyDo?

@artem-zinnatullin would you like to deprecate finallyDo -> doAfterTerminate in this PR?

[artem-zinnatullin]

I'd prepare a separate PR after this if you don't mind.

On Mon, Dec 7, 2015, 23:13 Aaron Tull notifications@github.com wrote:

Does anyone have any objection to creating Observable.doAfterTermination
as an alias for finallyDo and deprecating the original finallyDo?

@artem-zinnatullin https://github.com/artem-zinnatullin would you like
to deprecate finallyDo -> doAfterTerminate in this PR?

—
Reply to this email directly or view it on GitHub
#3434 (comment).

@artem_zin

[stealthcode]

Rebase please so we can merge.

[artem-zinnatullin]

Rebased!

[stealthcode]

👍

[akarnokd]

👍

[stevegury]

👍