-
Notifications
You must be signed in to change notification settings - Fork 170
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
chore: Convert to log/slog #519
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #519 +/- ##
==========================================
- Coverage 36.44% 36.33% -0.11%
==========================================
Files 29 29
Lines 3257 3275 +18
==========================================
+ Hits 1187 1190 +3
- Misses 1983 1998 +15
Partials 87 87 ☔ View full report in Codecov by Sentry. |
slog was introduced in Go 1.21, so this cannot be integrated yet since chamber still supports go 1.20. I'll leave this PR inactive until the 1.23 release, which is expected in August (about 2 months from now). |
The small amount of verbose logging in the exec command has been converted to debug logging under log/slog.
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.
moreso an open question, but thanks for doing this!
if verbose { | ||
fmt.Fprintf(os.Stdout, "info: With environment %s\n", strings.Join(env, ",")) | ||
} | ||
slog.Debug(fmt.Sprintf("info: With environment %s\n", strings.Join(env, ","))) |
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.
is there any world where this is considered a breaking change? It looks like the comment in the help output for verbose before was wrong since partially we output to STDERR (only this line apparently was to STDOUT)
Do we need to keep that consistency for any reason?
@bhavanki Let's merge this |
The small amount of verbose logging in the exec command has been
converted to debug logging under log/slog.