-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Mutable Documents #7722
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
Mutable Documents #7722
Conversation
aa3a900
to
3a8fbf4
Compare
Generated by 🚫 Danger |
026b8db
to
e8d9f0f
Compare
Coverage ReportAffected SDKs
Test Logs |
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.
We should also have some tests for this. Simply create documents, perform mutations and check they are in expected state should be good enough.
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.
Thanks for the review!
I didn't add tests since this will be exhaustively tested by the LocalStoreTests once I plug this into the rest of the system. The existing document types follow the same testing strategy :)
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.
LGTM!
This ports MutableDocument from Android: https://github.com/firebase/firebase-android-sdk/blob/master/firebase-firestore/src/main/java/com/google/firebase/firestore/model/MutableDocument.java