-
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
feat(NODE-4814): implement remaining severity logging methods #3629
feat(NODE-4814): implement remaining severity logging methods #3629
Conversation
23694d0
to
08638b4
Compare
1d7b3dc
to
40034c3
Compare
ab7b362
to
c2324c9
Compare
… of github.com:mongodb/node-mongodb-native into NODE-4814/implement_remaining_severity_logging_methods
Question: Do we want to remove the entries in the These would be:
|
Question: Do we want to have the helper that corresponds with the |
Agreed. I also think that these methods make the most sense in the verb form (i.e., "warn" instead of "warning")
I lean towards leaving all severity levels, for consistency with the spec. If we TS support to help enforce the lack of helpers for
|
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.
Tiny change
) { | ||
const severity = SEVERITY_LEVEL_MAP.getSeverityLevelName(l); | ||
for (let i = index; i >= 0; i--) { | ||
const severity = severities[i]; |
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.
if we type severities
as SeverityLevel[]
, we should be able to remove the cast in line 683
CI failures are unrelated. |
Description
What is changing?
Adds in the remainder of the severity loggers whose base designs were implemented in NODE-5169
Is there new documentation needed for these changes?
No
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript