-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
2.x: Evaluate Schedule initialization via Callable #4585
Merged
akarnokd
merged 13 commits into
ReactiveX:2.x
from
peter-tackage:fix/lazy-init-schedulers
Sep 26, 2016
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0ab5956
Evaluate Schedule initialization via Callable
5112117
Clarify docs that Schedulers are initialized by the return value of t…
85e14a9
Enforce non-null Callable Scheduler and Scheduler
1303f47
Add remaining tests and tidy
e521926
Expand relevant Javadoc
9ccf4ed
Make error messages more consistent
38bb3c0
Correct Exception naming
174ce7a
Add test for Exception message to verify root cause
aeafcc7
Add tests for alternative initialization path
786090d
Simplify statement
c191e31
Use holder pattern for default Scheduler instances
fff28b2
Use correct scheduler when verifying reset
a273507
Make onInitHandler functions lazy and enforce non null.
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not it throw on a null callable? What's the point of calling with null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was done for consistency with the existing expectations in
RxJavaPluginsTest.clearIsPassthrough()
, specifically:Should this be changed to only return null if the
Callable
returns null (and throw if theCallable
itself is null)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Null should not be allowed as a return value from the
Callable
nor from the initFunction
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a change, because previously the
Scheduler
value forRxJavaPlugins.init*
was allowed to be null, as per -assertNull(RxJavaPlugins.initSingleScheduler(null));
.I will add an additional set of tests for the new behavior (something along the lines of
assemblyHookCrashes
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.