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

Add to StorIO.LowLevel getter for underlying sqLiteOpenHelper #706

Merged
merged 2 commits into from
Feb 9, 2017

Conversation

nikitin-da
Copy link
Collaborator

Part of #704

@nikitin-da nikitin-da added this to the v1.12.0 milestone Oct 31, 2016
@nikitin-da nikitin-da self-assigned this Oct 31, 2016
@codecov-io
Copy link

codecov-io commented Oct 31, 2016

Codecov Report

Merging #706 into master will increase coverage by <.01%.

@@            Coverage Diff             @@
##           master     #706      +/-   ##
==========================================
+ Coverage   96.05%   96.05%   +<.01%     
==========================================
  Files          87       87              
  Lines        2459     2460       +1     
  Branches      329      267      -62     
==========================================
+ Hits         2362     2363       +1     
  Misses         68       68              
  Partials       29       29
Impacted Files Coverage Δ
.../com/pushtorefresh/storio/sqlite/StorIOSQLite.java 100% <ø> (ø)
...efresh/storio/sqlite/impl/DefaultStorIOSQLite.java 99.27% <100%> (ø)

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 d637f03...885d1d7. Read the comment docs.

Copy link
Member

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

Few small changes, otherwise LGTM!

@@ -329,6 +330,21 @@
* how to use this and when transactions are committed and rolled back.
*/
public abstract void endTransaction();

/**
* Returns {@link SQLiteOpenHelper} for the direct access to underlined database.
Copy link
Member

Choose a reason for hiding this comment

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

underlined -> underlying


/**
* Returns {@link SQLiteOpenHelper} for the direct access to underlined database.
* It can be used in cases when {@link StorIOSQLite} support doesn't provided.
Copy link
Member

Choose a reason for hiding this comment

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

when {@link StorIOSQLite} APIs are not enough. Sounds better to me :)

* Notice: Database changes through the direct access
* to the {@link SQLiteOpenHelper} will not trigger notifications.
* If it possible you should use {@link StorIOSQLite} methods instead
* or call {@link #notifyAboutChanges(Changes)}.
Copy link
Member

Choose a reason for hiding this comment

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

or call … manually.

@NonNull
@Override
public SQLiteOpenHelper sqliteOpenHelper() {
// doesn't required in design test
Copy link
Member

Choose a reason for hiding this comment

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

Not required in design test.

@@ -69,6 +69,15 @@ public void nullSQLiteOpenHelper() {
}

@Test
public void lowLevelReturnsSameInstanceOfSQLiteOpenHelper() {
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

👍

@nikitin-da
Copy link
Collaborator Author

@geralt-encore PTAL, please)

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.

Looks good!

@geralt-encore geralt-encore merged commit b9523f4 into master Feb 9, 2017
@geralt-encore geralt-encore deleted the issue-704 branch February 9, 2017 11:21
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.

4 participants