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
registry: configureLogging() simplify logic a bit #3772
registry: configureLogging() simplify logic a bit #3772
Conversation
thaJeztah
commented
Nov 4, 2022
- alternative to / closes fix unclear code comments #3670
Codecov ReportBase: 57.06% // Head: 56.92% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #3772 +/- ##
==========================================
- Coverage 57.06% 56.92% -0.14%
==========================================
Files 105 105
Lines 10806 10797 -9
==========================================
- Hits 6166 6146 -20
- Misses 3952 3962 +10
- Partials 688 689 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
registry/registry.go
Outdated
// Set the config to the default ("text") formatter in case none was | ||
// specified. | ||
config.Log.Formatter = "text" |
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.
oh! Wait, I had my wires crossed when I wrote this. I thought the original code (before my change) updated the config struct, so I added this line. I'm not a fan of updating the config itself, so let me move this back to the start (and use a local variable).
if config.Log.Formatter != "" { | ||
logrus.Debugf("using %q logging formatter", config.Log.Formatter) |
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.
.. because I think the "bug" was actually here; this block should've used the local variable (formatter
), not the config.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
7bf817c
to
b73c038
Compare
updated with the changes I mentioned above; PTAL 👍 |