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

Firestore: add support for 'ArrayRemove' / 'ArrayUnion' transforms #6651

Merged
merged 6 commits into from
Nov 27, 2018
Merged

Firestore: add support for 'ArrayRemove' / 'ArrayUnion' transforms #6651

merged 6 commits into from
Nov 27, 2018

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Nov 26, 2018 •
edited
Loading

Uses #6559 as a base. Please review that one first.

Closes #6546.

@tseaver tseaver added the api: firestore Issues related to the Firestore API. label Nov 26, 2018
@tseaver tseaver requested a review from crwilcox as a code owner November 26, 2018 21:34
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 26, 2018
@tseaver tseaver mentioned this pull request Nov 26, 2018
Closed
6 tasks
mcdonc
mcdonc approved these changes Nov 27, 2018
View reviewed changes
Copy link
Contributor

@mcdonc mcdonc left a comment

Choose a reason for hiding this comment

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

It's a big PR but I understand the component parts, and it looks good to me.

@tseaver tseaver mentioned this pull request Nov 27, 2018
Merged
@tseaver
Rename 'constants' module -> 'transforms'.
e784f60 
In preparation for adding 'ArrayUnion' / 'ArrayRemove' transforms, which
are not constants.

See: #6551.
@tseaver
Add 'ArrayUnion' transform.
c4a49d4 
Adds missing values to an array field.
@tseaver
Factor out '_ValueList' base class from 'ArrayUnion'.
cd400ba 
FBO adding 'ArrayRemove', which is functionally identical (only the class
docstring will vary).
@tseaver
Add 'ArrayRemove' transform.
039494a 
Removes values from an array field.
@tseaver
Convert 'ArrayRemove'/'ArrayUnion' to appropriate transform messages.
e27220b
@tseaver
Unblacklist conformance tests for 'ArrayRemove'/'ArrayUnion'.
cc312d9 
Closes #6546.
@tseaver
Copy link
Contributor Author

tseaver commented Nov 27, 2018

Rebased after merge of #6559. Will merge when CI is green.

@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 27, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 27, 2018
@tseaver tseaver merged commit 616255b into googleapis:master Nov 27, 2018
@tseaver tseaver deleted the 6546-firestore-array_remove-array_union branch November 27, 2018 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcdonc mcdonc mcdonc approved these changes

@crwilcox crwilcox Awaiting requested review from crwilcox

Assignees
No one assigned
Labels
api: firestore Issues related to the Firestore API. 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.
4 participants
@tseaver @mcdonc @googlebot @kokoro-team