-
Notifications
You must be signed in to change notification settings - Fork 66
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
feat: support slog attributes #127
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #127 +/- ##
==========================================
+ Coverage 80.92% 82.63% +1.71%
==========================================
Files 11 11
Lines 739 714 -25
==========================================
- Hits 598 590 -8
+ Misses 126 108 -18
- Partials 15 16 +1 ☔ View full report in Codecov by Sentry. |
I would say keep supporting Go 1.19 and split code. We can use type aliases to simplify things. For example, |
@aymanbagabas very cool idea! that worked pretty nice. |
02952f6
to
9e69d85
Compare
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.
Some nit picks and questions, looks good otherwise!
749f61b
to
ea97607
Compare
@aymanbagabas hi, did you see that all comments has been addressed? :) |
No worries. Good catch @aymanbagabas! Didn't realise the slog Fixing the text handler seem like a future improvement to me. Sorry, I don't have time to look at that. |
@aymanbagabas ping! 🐧 |
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, some last comments before merging
@aymanbagabas seems like govulncheck doesn't work with 1.19. Looks like golang/vuln@745db65 introduces |
Hmm, seems like golang/vuln needs to update their go.mod Anyway, this looks good, thanks again @op! |
This adds support for slog attributes and fixes #119.