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

Merged in fixes to OffsetDateTime by SamiKouatli. Fixed Andorid API'a before 26. #19

Closed
wants to merge 1 commit into from
Closed

Conversation

tkeithblack
Copy link
Contributor

I pulled this change from SamiKouatli's fork and have been using it for a bit over two months in a non-production product. It solved the pre-API 26 issue for me. I am submitting the pull request for consideration, however, I am not an expert in this area and cannot guarantee its effectiveness at all API levels. Feedback is appreciated.

Quoted from SamiKouatli's checkin:
OffsetDateTime is available only for sdk version > 26
This modification allows us to use the plugin with older version of Android. However it might not be the clean and proper way to do it.

@ericmartineau
Copy link
Contributor

This change was included in the 0.5.4 release.

@tkeithblack
Copy link
Contributor Author

Awesome, thanks!

@ericmartineau ericmartineau reopened this Aug 3, 2020
@ericmartineau
Copy link
Contributor

I'm looking further at it, and wondering why the PR used Instant. I'm not familiar with the specifics of Android versions, but Instant is a part of the java8 time spec, like OffsetDateTime. Are you saying it works for you like this?

@tkeithblack
Copy link
Contributor Author

@ericmartineau,

I have used this from my own fork on both Android API 24 & 28. It runs without errors and allows querying of contacts. However, I have very limited testing thus I'm not in a position to say it works w/o issues. Sorry I can't be of more help.

@ericmartineau
Copy link
Contributor

Either way, I introduced joda-time and got rid of the reference to Instant - I'm guessing it would've caused some issues.

@tkeithblack
Copy link
Contributor Author

Thanks, I'll pull it and give it a try!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants