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

Rewrite optional usage to allow Flowable implementation #856

Merged
merged 5 commits into from
Nov 18, 2017

Conversation

nikitin-da
Copy link
Collaborator

No description provided.

@nikitin-da nikitin-da added this to the v3.0.0 milestone Nov 16, 2017
@nikitin-da nikitin-da self-assigned this Nov 16, 2017
@codecov-io
Copy link

codecov-io commented Nov 16, 2017

Codecov Report

Merging #856 into master will increase coverage by 0.34%.
The diff coverage is 98.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #856      +/-   ##
============================================
+ Coverage     97.33%   97.68%   +0.34%     
- Complexity      799      819      +20     
============================================
  Files            92       97       +5     
  Lines          2663     2719      +56     
  Branches        298      299       +1     
============================================
+ Hits           2592     2656      +64     
+ Misses           44       35       -9     
- Partials         27       28       +1
Impacted Files Coverage Δ Complexity Δ
...ite/operations/get/PreparedGetNumberOfResults.java 100% <ø> (ø) 5 <0> (ø) ⬇️
...shtorefresh/storio2/sqlite/LoggingInterceptor.java 100% <ø> (ø) 4 <0> (ø) ⬇️
...operations/put/PreparedPutCollectionOfObjects.java 92.4% <ø> (ø) 6 <0> (ø) ⬇️
...ver/operations/get/PreparedGetNumberOfResults.java 100% <ø> (ø) 4 <0> (ø) ⬇️
...erations/put/PreparedPutContentValuesIterable.java 91.66% <ø> (ø) 6 <0> (ø) ⬇️
...esh/storio2/sqlite/operations/get/PreparedGet.java 100% <ø> (ø) 6 <0> (ø) ⬇️
...orio2/sqlite/operations/get/PreparedGetCursor.java 100% <ø> (ø) 5 <0> (ø) ⬇️
...tentresolver/operations/get/PreparedGetCursor.java 91.66% <ø> (ø) 4 <0> (ø) ⬇️
...sqlite/operations/delete/PreparedDeleteObject.java 100% <ø> (ø) 6 <0> (ø) ⬇️
...qlite/operations/delete/PreparedDeleteByQuery.java 100% <ø> (ø) 6 <0> (ø) ⬇️
... and 26 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 0643752...3ddfe27. Read the comment docs.

Copy link
Collaborator

@geralt-encore geralt-encore left a comment

Choose a reason for hiding this comment

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

Cool! Now I got the idea

try {
final Result value = preparedOperation.executeAsBlocking();
emitter.onNext(Optional.of(value));
emitter.onComplete();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure that it has to complete here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think yes. We use this flowable only for starting emission or if have no tables/tags to observe.
Another flowable created by ChangesFilter.applyForTablesAndTags will not complete

@@ -56,7 +56,7 @@
@WorkerThread
@NonNull
Copy link
Collaborator

@geralt-encore geralt-encore Nov 16, 2017

Choose a reason for hiding this comment

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

Nullable ?

}

private class RealCallInterceptor<UnwrappedResult> implements Interceptor {
private class RealCallInterceptor implements Interceptor {
@NonNull
Copy link
Collaborator

@geralt-encore geralt-encore Nov 16, 2017

Choose a reason for hiding this comment

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

Nullable

@nikitin-da
Copy link
Collaborator Author

I will add tests for RxJavaUtils tomorrow
Hope codecov take pity on me then)

@nikitin-da nikitin-da changed the title [draft]Rewrite optional usage to allow Flowable implementation Rewrite optional usage to allow Flowable implementation Nov 16, 2017
Copy link
Collaborator

@geralt-encore geralt-encore left a comment

Choose a reason for hiding this comment

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

Nice! I'll try to work on Maybe as soon as I can, but can't promise anything - too much stuff is going on =(

@geralt-encore geralt-encore merged commit 835ff7f into master Nov 18, 2017
@nikitin-da nikitin-da deleted the rewrite-optional-usage branch December 3, 2017 10:39
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