Skip to content

ObjectValue changes #8007

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 6 commits into from
May 10, 2021
Merged

Conversation

schmidt-sebastian
Copy link
Contributor

This PR is part of the larger series and renames MutableObjectValue to ObjectValue (the original ObjectValue is gone). It also adds support for unsorted MapValues, since the Watch backend does in fact send unsorted maps.

@google-cla google-cla bot added the cla: yes label Apr 29, 2021
EXPECT_EQ(WrapObject("a", "object_value"), object_value);
object_value.Set(Field("a"), Wrap("object_value"));
object_value.Set(Field("a"), Value("object_value"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: Value is the function name in master, so this actually reduces the diff.

@google-oss-bot
Copy link

1 Warning
⚠️ Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

@schmidt-sebastian schmidt-sebastian force-pushed the mrschmidt/ovchanges branch 2 times, most recently from 61e2c94 to 1479556 Compare May 3, 2021 15:50
Copy link
Contributor

@var-const var-const left a comment

Choose a reason for hiding this comment

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

Almost LGTM with one question.

/** `MutableObjectValue` models unique ownership. */
MutableObjectValue(const MutableObjectValue&) = delete;
MutableObjectValue& operator=(const MutableObjectValue&) = delete;
ObjectValue& operator=(const ObjectValue&) = delete;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this class copy constructible but not copy assignable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I would need this operator:

Message& operator=(const Message&) = delete;

Copy link
Contributor

Choose a reason for hiding this comment

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

Right.

@schmidt-sebastian
Copy link
Contributor Author

@var-const Can you take another look?

Copy link
Contributor

@var-const var-const left a comment

Choose a reason for hiding this comment

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

Sorry about the delay! LGTM.

/** `MutableObjectValue` models unique ownership. */
MutableObjectValue(const MutableObjectValue&) = delete;
MutableObjectValue& operator=(const MutableObjectValue&) = delete;
ObjectValue& operator=(const ObjectValue&) = delete;
Copy link
Contributor

Choose a reason for hiding this comment

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

Right.

@schmidt-sebastian schmidt-sebastian merged commit 330ed4a into mrschmidt/mutabledocuments May 10, 2021
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/ovchanges branch May 10, 2021 19:34
@firebase firebase locked and limited conversation to collaborators Jun 10, 2021
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