-
Notifications
You must be signed in to change notification settings - Fork 9.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
server: Simplify passing logger setup by passing only logger #12968
Conversation
Codecov Report
@@ Coverage Diff @@
## main #12968 +/- ##
==========================================
+ Coverage 62.73% 67.76% +5.02%
==========================================
Files 417 404 -13
Lines 33332 32750 -582
==========================================
+ Hits 20910 22192 +1282
+ Misses 10338 8584 -1754
+ Partials 2084 1974 -110
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
370f9f3
to
41ed748
Compare
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.
Nice!
Curious should we mention this in the release notes, as its an exposed server configuration, or what do we consider as public API breaking changes?
I think that exported properties of the 'embed' package has not changed, so its an |
If we are going to ignore the logging fields from |
embed.Config never accepted Lines 381 to 383 in ddc4f47
The way to configure it was to pass: Line 368 in ddc4f47
So the breaking change already happened in: eafbc8c where the signature got simplified. |
Brought back the original API (and changelog in #12973) |
Follow up from #12326
cc @ptabor