Skip to content

Conversation

@var-const
Copy link
Contributor

No description provided.

Copy link
Contributor

@zxu123 zxu123 left a comment

Choose a reason for hiding this comment

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

LGTM

Some thought no need to address in this PR: I saw the updating logic of change is complicated; which is ported directly from the objc code. It would help if we have some document (or embedded chart as comment) showing that the cases are comprehensive i.e. we did not miss any combination of the two types.

Copy link
Member

@rsgowman rsgowman left a comment

Choose a reason for hiding this comment

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

lgtm; sorry about the slow turnaround.

@var-const
Copy link
Contributor Author

It would help if we have some document (or embedded chart as comment) showing that the cases are comprehensive i.e. we did not miss any combination of the two types.

Agreed.

@var-const var-const merged commit 7cc0a9b into master Feb 25, 2019
@var-const var-const deleted the varconst/fst-view-snapshot2 branch February 25, 2019 19:10
@firebase firebase locked and limited conversation to collaborators Oct 21, 2019
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.

4 participants