-
Notifications
You must be signed in to change notification settings - Fork 943
rename shutdown to terminate and publish it #2116
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
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. One optional comment nit.
packages/firebase/index.d.ts
Outdated
* | ||
* Termination does not cancel any pending writes and any promises that are awaiting a response | ||
* from the server will not be resolved. If you have persistence enabled, the next time you | ||
* start this instance, it will resume attempting to send these writes to the server. |
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.
s/attempting to send/sending/
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.
Done.
packages/firebase/index.d.ts
Outdated
* After calling `terminate()` only the `clearPersistence()` method may be used. Any other method | ||
* will throw a `FirestoreError`. | ||
* | ||
* To restart after termination, simply create a new instance of FirebaseFirestore with |
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 strenuously avoid using this word (in case there are users who do not find it simple at all). Please remove.
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.
Done.
packages/firebase/index.d.ts
Outdated
* To restart after termination, simply create a new instance of FirebaseFirestore with | ||
* `firebase.firestore()`. | ||
* | ||
* Termination does not cancel any pending writes and any promises that are awaiting a response |
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.
Suggest a comma after "writes" to disambiguate.
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.
Done.
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.
A couple of style nits for you. Thanks!
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!
No description provided.