feat(NODE-4522)!: remove callback support #3499
Conversation
nbbeeken
force-pushed
the
NODE-4522-rm-callbacks
branch
2 times, most recently
from
d58f1c2 to
4ff1759
Compare
nbbeeken
force-pushed
the
NODE-4522-rm-callbacks
branch
from
4ff1759 to
8cd919d
Compare
nbbeeken
force-pushed
the
NODE-4522-rm-callbacks
branch
from
8cd919d to
217101d
Compare
src/bulk/common.ts
Outdated
| async execute(options?: BulkWriteOptions): Promise<BulkWriteResult> { | ||
| options ??= {}; |
There was a problem hiding this comment.
is it better to use default arg or coalesce assignment 🤔
There was a problem hiding this comment.
personal preference is always default arguments? but either is fine. I just prefer not to have to default ourselves - why not let JS do the work for us 🙂
There was a problem hiding this comment.
default arg is cleaner imho, but it honestly doesn't matter as long as we're consistent
There was a problem hiding this comment.
Made this consistent in the few places it wasn't
nbbeeken
changed the title
dariakp
added
the
Primary Review
|
our driver CI is "green", however the FLE integration tests that we pull in from the other repo have callback usage in the tests: mongodb/libmongocrypt#539 (this also makes socks-5 coverage fail b/c we test that FLE works through a proxy) |
|
CI run with the commit hash of the changes in this PR: mongodb/libmongocrypt#539
take2: https://spruce.mongodb.com/version/63ceec460305b9248e36396c/tasks |
Co-authored-by: Daria Pardue <daria.pardue@mongodb.com>
|
FLE run pointing at my PR mongodb/libmongocrypt#539 |
Description
What is changing?
What is the motivation for this change?
Promises provide a consistent way to handle the outcome of an asynchronous operation, unifying the API here to always use the
asyncsyntax forces the code to always return a promise so that errors/resolutions are always handled the same way.Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript