Fix a couple small issues I tripped over today. #2806
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Fix primeBackend so it uses an isolated FIRFirestore* instance. I ran into an issue where tests that try to change .settings would fail if they were the very first test run, since primeBackend had already configured the client and so settings could no longer be changed. 2. FIRFirestore.client was incorrectly typed / commented as non-null.
wilhuff
reviewed
Apr 11, 2019
Firestore/Source/API/FIRFirestore.mm
Outdated
| */ | ||
| - (FSTFirestoreClient *)client { | ||
| - (nullable FSTFirestoreClient *)client { |
There was a problem hiding this comment.
Is this actually desirable? The comment was wrong, but should anyone be accessing this before it's set up?
Why not assert that the client is non-null and then keep the nullability the way it was?
There was a problem hiding this comment.
Good question. Thanks for prodding me to be more holistic. :-) I ended up noticing that this is barely used and so I deleted it, but added the assert you wanted to api::Firestore.client() which was also missing it.
…ient is configured.
Corrob
pushed a commit
that referenced
this pull request
Apr 25, 2019
* Fix a couple small issues I tripped over today. 1. Fix primeBackend so it uses an isolated FIRFirestore* instance. I ran into an issue where tests that try to change .settings would fail if they were the very first test run, since primeBackend had already configured the client and so settings could no longer be changed. 2. Remove FIRFirestore.client and make api::Firestore.client() assert client is configured.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
an issue where tests that try to change .settings would fail if they were the
very first test run, since primeBackend had already configured the client and
so settings could no longer be changed.