Skip to content

2.x: cleanup, bugfixes, coverage 8/27-2 #4434

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 27, 2016

Conversation

akarnokd
Copy link
Member

  • Remove unused code
  • Improve coverage of Single, Completable and Observable
  • Fix minor bugs in operators

@@ -1743,16 +1743,10 @@ public static int bufferSize() {
*/
@SchedulerSupport(SchedulerSupport.CUSTOM)
public static Observable<Long> interval(long initialDelay, long period, TimeUnit unit, Scheduler scheduler) {
if (initialDelay < 0) {
initialDelay = 0L;
Copy link
Contributor

Choose a reason for hiding this comment

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

Surprised we aren't throwing here (and others)

throw new IllegalArgumentException("skip > 0 required but it was " + count);
}
verifyPositive(count, "count");
verifyPositive(skip, "skip");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think one of these is wrong because skip can be 0 but count cannot.

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on other methods in this diff I would guess verifyPositive rejects 0

Copy link
Member Author

Choose a reason for hiding this comment

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

It creates buffers every index % skip so zero not allowed.

@JakeWharton
Copy link
Contributor

👍

@codecov-io
Copy link

codecov-io commented Aug 27, 2016

Current coverage is 73.67% (diff: 92.59%)

Merging #4434 into 2.x will increase coverage by 2.74%

@@                2.x      #4434   diff @@
==========================================
  Files           453        453          
  Lines         32383      32373    -10   
  Methods           0          0          
  Messages          0          0          
  Branches       5233       5220    -13   
==========================================
+ Hits          22969      23852   +883   
+ Misses         7300       6437   -863   
+ Partials       2114       2084    -30   

Powered by Codecov. Last update 70d36fb...ff7d7ff

@akarnokd akarnokd merged commit 25e78c5 into ReactiveX:2.x Aug 27, 2016
@akarnokd akarnokd deleted the Coverage827_2 branch August 27, 2016 19:36
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