Skip to content
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

fix: make extra log arguments as attributes #19

Closed
wants to merge 1 commit into from

Conversation

violin0622
Copy link

It seems gorm slog changed log format, making those attributes as part of the message.
I dont' think this is good idea.
For example, This is what slog-gorm v1.3.0 does(when formatted by devslog handler):

截屏2024-03-21 11 39 55

With a previous version of slog-gorm, it looks like:
截屏2024-03-21 11 40 23

Definitely the second one is better.

So I make this PR to recover previous behaviour.

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (d05e02a) to head (027a8b0).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #19   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          152       154    +2     
=========================================
+ Hits           152       154    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orandin
Copy link
Owner

orandin commented Apr 3, 2024

Thank you @violin0622 for your contribution 👍

Your contribution only works for the Trace method. The methods Info, Warn & Error must use fmt.Sprintf to respect the Writer interface (#15). It seems that the PR #21 manage these different cases.

@orandin orandin added the bug Something isn't working label Apr 3, 2024
@orandin
Copy link
Owner

orandin commented Apr 4, 2024

Fixed by #21

@orandin orandin closed this Apr 4, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants