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

1.x: deprecate create(), add alternatives #5086

Merged
merged 1 commit into from
Feb 10, 2017

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Feb 9, 2017

Looks like create() won't go away unless we get the IDE mark it someway, such as being deprecated.

This PR deprecates create() and adds unsafeCreate for internal use and deprecate-renames fromEmitter to create(Action1, BackpressureMode).

There was an earlier attempt at deprecating create, #4253, but it was not followed up. This PR executes what I suggested in one of the comments.

@akarnokd akarnokd added this to the 1.3 milestone Feb 9, 2017
@codecov
Copy link

codecov bot commented Feb 9, 2017

Codecov Report

Merging #5086 into 1.x will decrease coverage by -0.01%.

@@             Coverage Diff              @@
##                1.x    #5086      +/-   ##
============================================
- Coverage     84.24%   84.24%   -0.01%     
  Complexity     2859     2859              
============================================
  Files           288      288              
  Lines         17805    17807       +2     
  Branches       2420     2420              
============================================
+ Hits          15000    15001       +1     
- Misses         1954     1955       +1     
  Partials        851      851
Impacted Files Coverage Δ Complexity Δ
...java/rx/plugins/RxJavaObservableExecutionHook.java 100% <ø> (ø) 6 <ø> (ø)
src/main/java/rx/observables/AsyncOnSubscribe.java 66.66% <ø> (ø) 5 <ø> (ø)
src/main/java/rx/observables/SyncOnSubscribe.java 91.85% <ø> (ø) 8 <ø> (ø)
src/main/java/rx/Single.java 70.08% <100%> (ø) 77 <1> (ø)
...ain/java/rx/observables/ConnectableObservable.java 100% <100%> (ø) 7 <1> (ø)
...in/java/rx/internal/operators/OperatorPublish.java 78.48% <100%> (ø) 8 <1> (ø)
...a/rx/internal/operators/EmptyObservableHolder.java 100% <100%> (ø) 3 <ø> (ø)
...internal/operators/OnSubscribeFlattenIterable.java 88.46% <100%> (ø) 4 <ø> (ø)
.../java/rx/internal/operators/OnSubscribeCreate.java 95.81% <100%> (ø) 6 <1> (?)
.../rx/internal/util/ScalarSynchronousObservable.java 90% <100%> (ø) 8 <1> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a37e292...a072448. Read the comment docs.

Copy link
Contributor

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

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

Excellent work!

* @see rx.functions.Cancellable
* @since 1.2.7 - experimental
*/
@Experimental
Copy link
Contributor

Choose a reason for hiding this comment

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

The lack of a stable fromEmitter and now two-argument create() is quite a burden on libraries. I would love to see this go to stable in 1.3 (skipping @Beta).

*/
@Experimental
@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -49,7 +49,7 @@ public void before() {

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this file needs renamed

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@akarnokd akarnokd merged commit 47e6c67 into ReactiveX:1.x Feb 10, 2017
@akarnokd akarnokd deleted the UnsafeCreate branch February 10, 2017 08:38
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.

2 participants