feat(NODE-4817)!: remove legacy logger #3518
Conversation
W-A-James
force-pushed
the
NODE-4817/remove-legacy-logger
branch
2 times, most recently
from
bb0ed91 to
055c4b3
Compare
durran
added
the
Primary Review
W-A-James
force-pushed
the
NODE-4817/remove-legacy-logger
branch
from
1bb04e7 to
f99a285
Compare
…ode-mongodb-native into NODE-4817/remove-legacy-logger
durran
left a comment
durran
left a comment
There was a problem hiding this comment.
More removals in comment responses.
durran
left a comment
durran
left a comment
There was a problem hiding this comment.
Just one last set of changes, thanks!
src/sdam/srv_polling.ts
Outdated
| this.logger.error(`Unexpected ${new MongoRuntimeError(unexpectedRuntimeError).stack}`); | ||
| }); | ||
| // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars | ||
| this._poll().catch(_unexpectedRuntimeError => {}); |
There was a problem hiding this comment.
From slack discussion, don't want to remove this as we don't want to have an unhandled promise, so we can just swallow it here.
There was a problem hiding this comment.
Can you change this to ()=>null it matches the rest of our catch and release promise handling.
There was a problem hiding this comment.
we can drop the underscore argument
|
https://spruce.mongodb.com/version/63caf4e1e3c3310b2e31edee/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC |
dariakp
removed
the
Primary Review
…ode-mongodb-native into NODE-4817/remove-legacy-logger
nbbeeken
changed the title
src/sdam/srv_polling.ts
Outdated
| } | ||
| } | ||
|
|
||
| // TODO(NODE-4817): This method has been left to allow for refactoring with the new logger |
There was a problem hiding this comment.
TODOs normally refer to tickets that haven't been done yet so we can find the relevant code again. I think this should probably be something in the new spec-ed logger epic
Description
Removing references to the legacy
Loggerclass in preparation for the implementation and installation of the newMongoLoggerIs there new documentation needed for these changes?
No
Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript