-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: use slog handler #17
Conversation
Using the handler directly allows to correctly set the Record.PC
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.
This fixes the issue raised but also eliminates the benefit of a structure logger as everything is now just a Sprintf
.
Is there a way where we can have Info
and Infof
etc?
We can do that, but this is targeting GORM lib, and they use their interface as if it was a Sprintf call. It would be on them to extend their Interface and add new methods. Why would you like to add methods that are not part of the interface? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 129 143 +14
=========================================
+ Hits 129 143 +14 ☔ View full report in Codecov by Sentry. |
Would be nice to support a more structured approach, but that would require internal changes to gorm, so I think this is the best that can be done without a breaking change there. |
@stevenh coverage improved :-) |
@orandin tagging you to get this rolling. Please let me know if you want something else changed. Thanks!! |
@injeniero thank you for your great contribution and @stevenh for your review 🥳 |
Using the handler directly allows to correctly set the Record.PC
This PR also fixes #15