-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: suppress debug logs when using -q #1254
Conversation
var logger swag.Debugger | ||
logger := log.New(os.Stdout, "", log.LstdFlags) |
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.
Moving initializing the debugger to here to avoid nil
confusions.
gen/gen.go
Outdated
@@ -117,6 +123,7 @@ type Config struct { | |||
|
|||
// Build builds swagger json file for given searchDir and mainAPIFile. Returns json. | |||
func (g *Gen) Build(config *Config) error { | |||
g.debug = config.Debugger |
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.
It would make more sense to initialize this in the New()
method. But since there are no other args being passed in there yet, I hesitated.
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.
debug.New() should init Gen struct with debug field of value log.New(os.Stdout, "", log.LstdFlags) so the old behavior wont break.
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.
Thanks @ubogdan! Fixed it now, looks like the tests are passing again.
log.Printf("Using overrides from %s", config.OverridesFile) | ||
g.debug.Printf("Using overrides from %s", config.OverridesFile) |
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.
log.Printf
outputs to stderr, whereas g.debug.Printf
will output either to stdout or be discarded.
Codecov Report
@@ Coverage Diff @@
## master #1254 +/- ##
=======================================
Coverage 94.99% 95.00%
=======================================
Files 14 14
Lines 2618 2620 +2
=======================================
+ Hits 2487 2489 +2
Misses 72 72
Partials 59 59
Continue to review full report at Codecov.
|
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
(Whoops, hadn't noticed this was already merged when I requested review 😄, thanks! ) |
Describe the PR
Fixes an issue where certain debug logs were always directed to
stderr
, even if the-q
flag was provided.Relation issue
#1255
Additional context
This fix will be helpful for anyone who wants to suppress all debug output (e.g. when generating API docs as part of a git commit hook).
I took the path of least resistance here since I'm not sure on the development philosophy for this project. For example, I initialized the new
Gen.debug
field in theBuild()
method instead of theNew()
method. Happy to pass the debugger in as an arg toNew()
instead if that's preferred.I did run the changes and they work as expected.