make FIRDocumentListenOptions.includeMetadataChanges private #540
Conversation
as discussed in Firestore API discussion, the name clashes creates confusion. Android SDK already not exposing the property; here change iOS SDK to match it.
|
This is currently public, right? If so, it's a breaking change and can't be submitted until the next full version bump of the Firebase SDK. |
|
Now merges with firestore-api-changes for next version. |
|
@ryanwilson Firestore is still in beta, though, and in the past has (after alpha) made breaking changes without a major version bump. |
|
@morganchen12 Ahhh right! I forgot it wasn't at 1.0 yet. Sorry about that! I'm good with this going into master then if desired. |
|
We're not yet merging to master because we're attempting to batch up a bunch of API changes into a single release rather than dribbling them out over time and making every upgrade painful. We'll merge firestore-api-changes to master when we're ready to release them all. |
Firestore/CHANGELOG.md
Outdated
| @@ -1,5 +1,7 @@ | |||
| # Unreleased | |||
| - [changed] Firestore no longer has a direct dependency on FirebaseAuth. | |||
| - [changed] property includeMetadataChanges is now hidden in FIRDocumentListenOptions | |||
There was a problem hiding this comment.
From a user's point of view it doesn't really matter that this is now an internal property. From their point of view we just removed it.
This would be better phrased as:
Removed the includeMetadataChanges property in FIRDocumentListenOptions to avoid confusion with the factory method of the same name.
|
Should I merge with master or with firebase-ios-sdk? |
|
This should be squashed and merged into the firebase-api-changes branch since it's one of the breaking changes we're batching up. |
To avoid surprise by adding a warning. Right now, when offline, empty result is returned with no log nor error, see https://groups.google.com/forum/#!topic/google-cloud-firestore-discuss/puFl9HVU57I/discussion
* Move the logic from FSTEventManager to FSTRemoteStore. Pros: per query vs per stream. * Since state can go into unknown normally (e.g. no listener is registered and nobody care the status), we only warns when status is actually offline to avoid confusion.
All projects are now ExternalProjects This makes it much easier to build them all in a single pass.
as discussed in Firestore API discussion, the name clashes creates confusion. Android SDK already not exposing the property; here change iOS SDK to match it.
…e#540) * make FIRDocumentListenOptions.includeMetadataChanges private as discussed in Firestore API discussion, the name clashes creates confusion. Android SDK already not exposing the property; here change iOS SDK to match it. * update CHANGELOG
Discussion
As discussed in Firestore API discussion, the name clashes creates confusion. Android SDK already not exposing the property; here change iOS SDK to match it.
Testing
Tests pass.
API Changes
Its an implementation oversight. Fix to match Android SDK.