Skip to content

Remove Mutation tombstones - #1354

Merged
schmidt-sebastian merged 2 commits into
masterfrom
mrschmidt-removetombstone
Nov 5, 2018
Merged

Remove Mutation tombstones#1354
schmidt-sebastian merged 2 commits into
masterfrom
mrschmidt-removetombstone

Conversation

@schmidt-sebastian

Copy link
Copy Markdown
Contributor

If my understanding is correct, we no longer remove mutation batches from any place but the front of the queue. This PR removes the code that handles this removal.

@schmidt-sebastian
schmidt-sebastian force-pushed the mrschmidt-removetombstone branch from 2956645 to 2146dc9 Compare November 3, 2018 00:24

@wilhuff wilhuff left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM


/** Converts this batch into a tombstone */
toTombstone(): MutationBatch {
return new MutationBatch(this.batchId, this.localWriteTime, []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we restore the assertion that the mutations array is non-empty in the constructor?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@schmidt-sebastian
schmidt-sebastian merged commit 9d6917e into master Nov 5, 2018
@schmidt-sebastian
schmidt-sebastian deleted the mrschmidt-removetombstone branch March 12, 2019 23:30
@firebase firebase locked and limited conversation to collaborators Oct 15, 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.

3 participants