-
-
Notifications
You must be signed in to change notification settings - Fork 466
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
[WIP] Replace log with logrus #112
Conversation
Signed-off-by: Nicolas Lamirault <[email protected]>
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.
Finally! Thanks a lot for starting work on this :)
I have some suggestions here:
- You could import logrus like this:
log "github.com/sirupsen/logrus"
and keep on usinglog.XYZ
instead of replacing every instance oflog
withlogrus
- Everywhere, where the logged message starts with a "level" like "ERROR:", "WARN:", "DEBUG:", or something the like, you could replace the
log.Println
with the appropriate levelled logger of logrus, e.g.log.Debugln
orlog.Errorf
and so on.
Thanks again, I really like to see the logging improving 👍
Signed-off-by: Nicolas Lamirault <[email protected]>
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! Thank you very much for your contribution :)
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Thank you @nlamirault ! |
See #17