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.

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

fix: recursive delete: backporting changes from Java #1514

Merged
merged 3 commits into from May 26, 2021

Conversation

thebrianchen
Copy link

Backporting some changes from the Java port

@thebrianchen thebrianchen self-assigned this May 25, 2021
@thebrianchen thebrianchen requested review from a team as code owners 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
private started = false;

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

// 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
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
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 join this conversation on GitHub. Already have an account? Sign in to comment
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.

None yet

2 participants