Fix destructuring for getAll#502
Fix destructuring for getAll#502ushu wants to merge 1 commit intogoogleapis:masterfrom ushu:fix-getAll-destructuring
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
|
I signed the CLA docs |
|
CLAs look good, thanks! |
|
Hi @ushu! Thanks for doing this! While this should work for TypeScript users, it would be nice if we also cleaned up the SDK code itself (https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/index.ts#L707 and https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/transaction.ts#L161). Do you want to take a stab at that as well? Let us know if you are busy and we can take over :) Note that the original intent of this signature was that to make it clear that the order of arguments is "DocumentReferences followed by field mask". This is probably not worth breaking our users over though... Thanks for catching this! |
|
Superseded by #515. |
This is a proposal to fix #501 : it literally restores the previous array-based signature, while keeping support for
ReadOptions-typed arguments.