Skip to content

Allowing empty merge calls#144

Merged
schmidt-sebastian merged 3 commits intomasterfrom
emptymerge
Feb 26, 2018
Merged

Allowing empty merge calls#144
schmidt-sebastian merged 3 commits intomasterfrom
emptymerge

Conversation

@schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Feb 26, 2018

This PR allows the call set({}, merge:true) since it can be used to ensure that a document exist on the server.

This came up as a valid use case in an internal discussion (b/73495873 - based on customer feedback in b/73132705).

There is currently a conformance test that rejects this call. I excluded the test and will work with @jba on an update.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 26, 2018
@codecov
Copy link

codecov bot commented Feb 26, 2018

Codecov Report

Merging #144 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #144   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines        1728   1729    +1     
=====================================
+ Hits         1728   1729    +1
Impacted Files Coverage Δ
src/write-batch.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f23c167...337d5f7. Read the comment docs.

Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

validate.isDocumentReference('documentRef', documentRef);
validate.isDocument('data', data, {
allowEmpty: !merge,
allowEmpty: true,

This comment was marked as spam.

This comment was marked as spam.

@schmidt-sebastian schmidt-sebastian merged commit ef2730d into master Feb 26, 2018
@schmidt-sebastian schmidt-sebastian deleted the emptymerge branch June 26, 2018 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants