-
Notifications
You must be signed in to change notification settings - Fork 617
Move DocumentId to public space #571
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
/test device-check-changed |
8c0e7da
to
ad38eb4
Compare
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 with nits. I made some suggestions to the javadoc, but I know this got some scrutiny in the API review already. If you feel like my feedback is contrary to feedback you got previously, feel free to punt.
firebase-firestore/src/androidTest/java/com/google/firebase/firestore/POJOTest.java
Outdated
Show resolved
Hide resolved
firebase-firestore/src/androidTest/java/com/google/firebase/firestore/POJOTest.java
Show resolved
Hide resolved
firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentId.java
Outdated
Show resolved
Hide resolved
firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentId.java
Outdated
Show resolved
Hide resolved
firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentId.java
Outdated
Show resolved
Hide resolved
firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentId.java
Outdated
Show resolved
Hide resolved
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.
Addressed your comments, PTAL.
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.
Suuper close, but I just remembered that we should add a firebase-firestore/CHANGELOG.md entry for this now that it's going to be public (which will then get pulled into https://firebase.google.com/support/release-notes/android). Sorry!
CHANGELOG added. |
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 good!
* Move DocumentId to public space * add nest object testing * add changelog
No description provided.