-
Notifications
You must be signed in to change notification settings - Fork 640
firestore: Instant
support
#6235
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
Conversation
Following up. @tom-andersen, I just realized you requested that I tag you here. |
@tom-andersen, the latest test failures seem to be unrelated to my change, and this branch is fully caught-up. |
Thanks! I will copy the branch over to make the CI run properly. I will also need to understand how this is backward compatible. |
Thanks.
The same way googleapis/java-firestore#1586 was. |
Consider adding support for https://kotlinlang.org/api/kotlinx-datetime/kotlinx-datetime/kotlinx.datetime/-instant/ |
Done. Also added tests. |
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.
Looks great. Thank you for this contribution!
I don't follow the check failures... |
Ahh yes, don't worry about those. Those checks are broken on external PRs. But that does remind me... can you add an entry to the "Unreleased" section of
|
Done. |
…roken by #6235 The test failure fixed by this PR is: ``` com.google.firebase.firestore.ServerTimestampTest > testPOJOWithWrongType[emulator-5554 - 12] FAILED org.junit.ComparisonFailure: expected:<...ring instead of Date[ or Timestamp].> but was:<...ring instead of Date[, Timestamp, or Instant].> at org.junit.Assert.assertEquals(Assert.java:117) ``` It simply updates the expected exception message as follows: ``` - "java.lang.String instead of Date or Timestamp.", + "java.lang.String instead of Date, Timestamp, or Instant.", ```
Add support for
java.time.Instant
as a timestamp type.This is a port of googleapis/java-firestore#1586.