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

Update the SQLite dependency to support Android 7 #317

Merged
merged 1 commit into from
Oct 10, 2016

Conversation

flagbug
Copy link
Member

@flagbug flagbug commented Oct 8, 2016

/cc @ghuntley @ericsink Can you review this please?

@ericsink
Copy link
Contributor

ericsink commented Oct 8, 2016

Re: The failure of the automatic checks: It looks to be a problem with a missing Android API in the SDK on the build machine. If that's related to SQLitePCL.raw, I do not yet see how.

Re: The PR itself:

If you have a specific preference for using the system SQLite [only] on iOS, then it looks fine. Otherwise, I suggest using bundle_e_sqlite3 instead of bundle_green.

Did the unit tests pass for you with this change? Just curious -- @ghuntley had a problem with that when he attempted something similar.

@flagbug
Copy link
Member Author

flagbug commented Oct 8, 2016

Re: The failure of the automatic checks: It looks to be a problem with a missing Android API in the SDK on the build machine. If that's related to SQLitePCL.raw, I do not yet see how.

@ghuntley Could you take a look at that?

If you have a specific preference for using the system SQLite [only] on iOS, then it looks fine. Otherwise, I suggest using bundle_e_sqlite3 instead of bundle_green.

Alright, I can do that, I just had a look Azure Mobile SDK at what they are doing, and it seems like they're using green

Did the unit tests pass for you with this change? Just curious -- @ghuntley had a problem with that when he attempted something similar.

Yup, they're working now, I think @ghuntley just forgot to update the dependencies of the test project, so it still used the old raw version

@ghuntley
Copy link
Member

ghuntley commented Oct 9, 2016

Re: The failure of the automatic checks: It looks to be a problem with a missing Android API in the SDK on the build machine. If that's related to SQLitePCL.raw, I do not yet see how.
@ghuntley Could you take a look at that?

The CI failure is unrelated to this PR; this PR is good to merge once we have some clarification from Eric.

If you have a specific preference for using the system SQLite [only] on iOS, then it looks fine. Otherwise, I suggest using bundle_e_sqlite3 instead of bundle_green.
Alright, I can do that, I just had a look Azure Mobile SDK at what they are doing, and it seems like they're using green

Ugh damn; I was wanting to use bundle_e_sqlite3 on iOS for consistency in bug reports across all platforms but looks like Azure Mobile SDK has tied our hands in this matter?

@ericsink can share some insights into what it means for the ecosystem? What happens if we choose bundle_e_sqlite3 and Azure Mobile SDK is used within the same application as Akvache on iOS.

@ericsink
Copy link
Contributor

ericsink commented Oct 9, 2016

I dislike the idea that all SQLitePCL.raw-using libs have to choose the same bundle. That should not be necessary.

And if it were necessary, I would dislike it even more if bundle_green were the one everyone chose.

Use bundle_e_sqlite3.

But, to answer your question:

If two libs are using two different bundles, the last call to Batteries.Init() wins. In the example you gave, this could mean that Akavache might end up using the iOS system SQLite, or that Azure Mobile SDK might end up using e_sqlite3, neither of which would be a tragedy.

If either "last call wins" or "first call wins" were clearly more correct, I would choose it, but this choice seems arbitrary.

@ghuntley
Copy link
Member

Switched to bundle_e_sqlite3 and the unit tests are passing. Thank-you Eric for your on-going help.

image

@ghuntley ghuntley merged commit da17b3f into akavache5-master Oct 10, 2016
@ghuntley ghuntley deleted the update-sqlite branch October 10, 2016 06:27
@ghuntley ghuntley modified the milestone: 5.0.0 Nov 4, 2016
@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants