- Notifications
You must be signed in to change notification settings - Fork 50
chore: refactor diag logger #9
Conversation
Codecov Report
@@ Coverage Diff @@ ## main #9 +/- ## ========================================== - Coverage 94.27% 93.77% -0.50% ========================================== Files 35 37 +2 Lines 489 466 -23 Branches 78 75 -3 ========================================== - Hits 461 437 -24 - Misses 28 29 +1
Continue to review full report at Codecov.
|
| @MSNev I can't add you as a reviewer because it seems you aren't a member of the OpenTelemetry community? |
The looping might trim a tiny bit off, but you also add the array itself, the log level map, and the import statements. I think the readability alone is a good argument to keep it this way, but the minification win just isn't big enough IMO. |
obecny left a comment
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.
lgtm
MSNev left a comment
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.
looks good -- just added 1 suggestion
|
/cc @MSNev
Per request of @Flarna this is the diag refactoring split out of #6. There may be some relevant discussion in the previous PR.
This PR is a prerequisite for #10
setLogLevelis now removed in favor of a second optional argument tosetLoggersetLogger()/setLogger(undefined)/setLogger(null)are now disallowed and replaced withdisablediag,DiagLogger,DiagLogLevel, andLogFunction.