Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

fix: recursive delete: backporting changes from Java#1514

Merged
thebrianchen merged 3 commits intomasterfrom
bc/rc-backport
May 26, 2021
Merged

fix: recursive delete: backporting changes from Java#1514
thebrianchen merged 3 commits intomasterfrom
bc/rc-backport

Conversation

@thebrianchen
Copy link
Copy Markdown

Backporting some changes from the Java port

@thebrianchen thebrianchen self-assigned this May 25, 2021
@thebrianchen thebrianchen requested review from a team May 25, 2021 20:26
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 25, 2021
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label May 25, 2021
@thebrianchen thebrianchen changed the title Bc/rc backport fix: recursive delete: backporting changes from Java May 25, 2021
Comment thread dev/src/recursive-delete.ts Outdated
private started = false;

/** Query limit to use when fetching all descendants. */
private maxPendingOps: number;
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.

Does this need to be marked "private" (here and below)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for catching, done.

Comment thread dev/test/recursive-delete.ts Outdated
// This deferred completes when the second query is run.
const secondQueryDeferred = new Deferred<void>();

const firstStream = Array.from(Array(maxPendingOps).keys()).map((_, i) =>
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.

Can you create a helper variable for Array.from(Array(maxPendingOps).keys()) and name it appropriately? It is used twice and not very pretty :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: firestore Issues related to the googleapis/nodejs-firestore API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants