Skip to content

1.x: Add extend() for Single and Completable. #4419

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

Closed
wants to merge 1 commit into from
Closed

1.x: Add extend() for Single and Completable. #4419

wants to merge 1 commit into from

Conversation

JakeWharton
Copy link
Contributor

Matches Observable.extend(). These are all @Experimental (including Observable). Perhaps we want to rename them all to() to match 2.x?

@codecov-io
Copy link

codecov-io commented Aug 24, 2016

Current coverage is 84.29% (diff: 100%)

Merging #4419 into 1.x will decrease coverage by 0.15%

@@                1.x      #4419   diff @@
==========================================
  Files           270        272     +2   
  Lines         17516      17528    +12   
  Methods           0          0          
  Messages          0          0          
  Branches       2677       2677          
==========================================
- Hits          14792      14775    -17   
- Misses         1869       1898    +29   
  Partials        855        855          

Powered by Codecov. Last update a5c9453...ac3c291

* @since (if this graduates from Experimental/Beta to supported, replace this parenthetical with the release number)
*/
@Experimental
public <R> R extend(Func1<? super CompletableOnSubscribe, ? extends R> conversion) {
Copy link
Member

Choose a reason for hiding this comment

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

The problem with this is that CompletableOnSubscribe is practucally useless; you'll have to call Completable.create() with it to get meaningful opererations. The whole method should be the same as to in 2.x. The Observable.extend was pushed hard back then disergarding my reservations.

Of course if you can give a compelling case why reduce the options for the conversion, I'm open minded.

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 agree, and would much prefer to()'s semantics.

Since extend() is still @Experimental we have that option...

Copy link
Member

Choose a reason for hiding this comment

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

👍 sounds much better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Closing and will make a new PR, since it might be a day or two.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Successfully merging this pull request may close these issues.

3 participants