-
Notifications
You must be signed in to change notification settings - Fork 400
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
Add SlogLoggerAdapter #355
Conversation
Standard library is going to merge an experimental structured logging package `slog` soon. Therefore, it is important to support it.
Thanks @dkotik , works fine for me. |
@GreyXor ok bumped the version up and switched over to |
@dkotik gentle ping 🏓 |
Dear friend, I am not a maintainer here. I don't have the merge powers. What do I need to do? |
@dkotik Greetings! I'm not requesting a merge at this time; rather, I'm reaching out for a code review. As you can observe, I've provided four suggestions for enhancing your code. Could you please take a moment to review them? |
I am confused. I am a Github newb, or there are something with permissions - I do not see any review comments in the code itself or in the list of "Reviewers" to the top right of this. |
Thanks for your input, now you should be able to see it. |
LGTM, very good thanks! |
@dkotik can you run |
result := make([]any, 0, len(fields)*2) | ||
|
||
for key, value := range fields { | ||
// result = append(result, slog.Any(key, value)) |
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.
Can we now drop the comment?
Hey @dkotik, LGTM! Thanks for the contribution! I'll address #355 (comment) and fix CI on the |
Standard library is going to merge an experimental structured logging package
slog
soon. Therefore, it is important to support it.Happy to have this parked until /exp/slog is merged into standard library. I am using the adapter in production already.