Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[feature] Adding FieldValue.increment() #444

Merged
merged 14 commits into from
Mar 7, 2019
Merged

[feature] Adding FieldValue.increment() #444

merged 14 commits into from
Mar 7, 2019

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Nov 10, 2018 •
edited
Loading

This adds support for the new NUMERIC_ADD Field Transform.

There is a lot of error message cleanup in this PR to make the error message for specifying an invalid NumericAdd operand make sense.

DO NOT MERGE until backend support is available (at which point the system test should pass).

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 10, 2018
var-const
var-const approved these changes Nov 12, 2018
View reviewed changes
types/firestore.d.ts Outdated
* that tells the server to add the given value to the field's current
* value.
*
* If the either current field value or the operand uses floating point

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dev/system-test/firestore.ts Outdated
@@ -287,6 +287,20 @@ describe('DocumentReference class', () => {
});
});

it('supports numericAdd()', () => {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@schmidt-sebastian schmidt-sebastian changed the title Adding Numeric Add DO NOT MERGE Adding Numeric Add Nov 12, 2018
@schmidt-sebastian
Copy link
Contributor Author

FYI: Pushed commit to rename numericAdd() to increment().

@schmidt-sebastian schmidt-sebastian mentioned this pull request Nov 19, 2018
Merged
@schmidt-sebastian
Copy link
Contributor Author

PR updated to use increment() name in Proto.

@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Feb 7, 2019
@JustinBeckwith JustinBeckwith added needs work This is a pull request that needs a little love. status: blocked Resolving the issue is dependent on other work. labels Feb 9, 2019
@yoshi-automation yoshi-automation removed the 🚨 This issue needs some love. label Feb 14, 2019
@schmidt-sebastian schmidt-sebastian changed the title DO NOT MERGE Adding Numeric Add Adding Numeric Add Mar 6, 2019
@codecov
Copy link

codecov bot commented Mar 6, 2019

Codecov Report

Merging #444 into master will increase coverage by 0.2%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #444     +/-   ##
=========================================
+ Coverage   95.77%   95.98%   +0.2%     
=========================================
  Files          24       24             
  Lines        1943     1943             
  Branches      168      169      +1     
=========================================
+ Hits         1861     1865      +4     
+ Misses         59       57      -2     
+ Partials       23       21      -2
Impacted Files Coverage Δ
dev/src/validate.ts 94% <100%> (+4%) ⬆️
dev/src/index.ts 98.17% <100%> (ø) ⬆️
dev/src/reference.ts 99.68% <100%> (ø) ⬆️
dev/src/transaction.ts 98.33% <100%> (ø) ⬆️
dev/src/field-value.ts 95% <100%> (+1.25%) ⬆️
dev/src/v1/firestore_client.js 76.08% <0%> (ø) ⬆️
dev/src/watch.ts 99.04% <0%> (+0.85%) ⬆️

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 04eb91c...88bb4a0. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 6, 2019 •
edited
Loading

Codecov Report

Merging #444 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #444      +/-   ##
=========================================
+ Coverage   95.77%   95.9%   +0.12%     
=========================================
  Files          24      24              
  Lines        1943    1955      +12     
  Branches      168     169       +1     
=========================================
+ Hits         1861    1875      +14     
  Misses         59      59              
+ Partials       23      21       -2
Impacted Files Coverage Δ
dev/src/validate.ts 94% <100%> (+4%) ⬆️
dev/src/index.ts 98.17% <100%> (ø) ⬆️
dev/src/reference.ts 99.68% <100%> (ø) ⬆️
dev/src/transaction.ts 98.33% <100%> (ø) ⬆️
dev/src/field-value.ts 95% <100%> (+1.25%) ⬆️

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 1b6c7b1...bbaab30. Read the comment docs.

@schmidt-sebastian schmidt-sebastian removed status: blocked Resolving the issue is dependent on other work. needs work This is a pull request that needs a little love. labels Mar 6, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 6, 2019
@schmidt-sebastian schmidt-sebastian added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 6, 2019
@kokoro-team kokoro-team removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Mar 6, 2019
@schmidt-sebastian schmidt-sebastian merged commit 9f4c32c into master Mar 7, 2019
@schmidt-sebastian schmidt-sebastian changed the title Adding Numeric Add [feature] Adding Numeric Add Mar 7, 2019
@schmidt-sebastian schmidt-sebastian changed the title [feature] Adding Numeric Add [feature] Adding FieldValue.increment() Mar 7, 2019
schmidt-sebastian added a commit that referenced this pull request Mar 11, 2019
@schmidt-sebastian
Adding Numeric Add (#444)
9c9ce4c
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt-numericadd branch March 11, 2019 03:08
@yoshi-automation yoshi-automation removed the 🚨 This issue needs some love. label Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@var-const var-const var-const approved these changes

Assignees

@schmidt-sebastian schmidt-sebastian

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@schmidt-sebastian @var-const @JustinBeckwith @googlebot @kokoro-team @yoshi-automation