-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
refactor(NODE-5352): refactor AbstractOperation to use async #3729
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@malikj2000 There are test failures to address
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the comments below, there are also some failing unit tests for operations that subclass AbstractCallbackOperation
and eslint errors to address.
changed all operations to extend AbstractCallbackOperation changed all operations from execute to executeCallback
changed async tests to await
changed all operations to extend AbstractCallbackOperation changed all operations from execute to executeCallback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waiting on CI after the rebase before merging but otherwise LGTM
Description
What is changing?
created callback
subclass of operation
and changed execute
Is there new documentation needed for these changes?
no
What is the motivation for this change?
first step in asyncifying the operations layer
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript