-
Notifications
You must be signed in to change notification settings - Fork 898
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
Firestore: use string values for TargetPurpose enum #7257
Conversation
…rather than numeric
|
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (283,134 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
Note: The "Test Firestore" and "Test Firestore Integration" checks are failing and will be fixed by #7244. The failing tests are:
|
…EnumValues_PR7257
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
Firestore: Use string values for
TargetPurpose
enum, rather than numeric.Although string values can assist with debugging, the main purpose is so that the spec tests will generate JSON for the Android and iOS SDKs that is stable and readable.
A previous PR, #7229, added
TargetPurpose.ExistenceFilterMismatchBloom
which causedTargetPurpose.LimboResolution
to change its value from2
to3
. This did not cause any problems in this SDK; however, it broke the spec tests in the Android SDK (and probably the ios sdk too). By hardcoding the values as strings it is both more readable and more stable.Android port: firebase/firebase-android-sdk#4931
iOS port: firebase/firebase-ios-sdk#11191