Skip to content

2.x: coverage, fixes, cleanup 8/27-1 #4431

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

Merged
merged 1 commit into from
Aug 26, 2016
Merged

Conversation

akarnokd
Copy link
Member

  • updated Single.cache() to be lock-free, to allocate less and work properly
  • added test coverage to some classes
  • remove unused internal EmptyObserver
  • update PublishSubject to support cross-cancellation
  • update TestHelper.race to use the current thread for one of the tasks

@akarnokd akarnokd added this to the 2.0 RC 2 milestone Aug 26, 2016
@codecov-io
Copy link

codecov-io commented Aug 26, 2016

Current coverage is 70.90% (diff: 87.77%)

Merging #4431 into 2.x will increase coverage by 0.51%

@@                2.x      #4431   diff @@
==========================================
  Files           454        453     -1   
  Lines         32371      32383    +12   
  Methods           0          0          
  Messages          0          0          
  Branches       5223       5233    +10   
==========================================
+ Hits          22786      22962   +176   
+ Misses         7447       7303   -144   
+ Partials       2138       2118    -20   

Powered by Codecov. Last update 0094304...4cce05b


@Override
public void onSubscribe(Disposable d) {
// not supported by this operator
Copy link
Contributor

Choose a reason for hiding this comment

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

throw assertion error to ensure it never happens?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't support dispose() once the cache has subscribed but the upstream should still run - this method is a no-op. Throwing here is not allowed anyway.

However, I'm willing to extend the SingleCache (and the others) by implementing Disposable and people can then cast and cancel a cache() too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. I don't think that's worth it. I just misunderstood the comment and thought "not allowed" instead of "not supported".

@JakeWharton
Copy link
Contributor

👍

@akarnokd akarnokd merged commit 4c0f93b into ReactiveX:2.x Aug 26, 2016
@akarnokd akarnokd deleted the Coverage827_1 branch August 26, 2016 22:12
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.

3 participants