Skip to content

Add WriteBatch #3152

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

Merged
merged 89 commits into from
Jun 5, 2020
Merged

Add WriteBatch #3152

merged 89 commits into from
Jun 5, 2020

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Jun 2, 2020

Note: This is much easier to review with the whitespace-ignored diff - https://github.com/firebase/firebase-js-sdk/pull/3152/files?w=1

Added the terminate logic to Datastore to ensure that WriteBatches cannot be committed after terminate is called.
Comment on lines 79 to 87
const parsed = isMerge(options)
? this._dataReader.parseMergeData('WriteBatch.set', convertedValue)
: isMergeFields(options)
? this._dataReader.parseMergeData(
'WriteBatch.set',
convertedValue,
options.mergeFields
)
: this._dataReader.parseSetData('WriteBatch.set', convertedValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment from a previous PR. Can you convert this to if/else or factor it out into a function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@schmidt-sebastian schmidt-sebastian changed the base branch from mrschmidt/updatedoc to master June 5, 2020 05:56
@schmidt-sebastian schmidt-sebastian merged commit 6cd5ed1 into master Jun 5, 2020
@firebase firebase locked and limited conversation to collaborators Jul 6, 2020
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/writebatch branch July 9, 2020 17:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants