-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use slog in place of waterlog #14
Conversation
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, though the changes in config.load.go
look like a bit more than just changing out the logger. Change scope is something to watch out for in the future, because it makes it harder to review and harder to debug later to figure out what changed caused an issue.
I agree with you, but the code was way more duplicated than I could tolerate 😬 I can split the commit anyway. |
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.
Great work! I have a few relatively minor comments, but it looks fine otherwise 🙂
All sorted, I also squashed the commits. |
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, thanks!
slog is the new structured logging library from the Go project itself. It is available as an experimental package for Go 1.20 and was upstreamed in Go 1.21. This makes it a perfect candidate for our tooling, because:
This PR only replaces waterlog with slog, using its TextHandler Handler. TextHandler is nowhere near the look of waterlog, but a custom Handler mimicking the old look is in the works and will come with a different PR in the near future.