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

replace direct use of logrus? #140

Open
mmlb opened this issue Apr 29, 2024 · 1 comment
Open

replace direct use of logrus? #140

mmlb opened this issue Apr 29, 2024 · 1 comment
Labels
v2 Breaking change appropriate for ironlib v2

Comments

@mmlb
Copy link
Member

mmlb commented Apr 29, 2024

Opening this up here to have a single place for discussion regarding this. I've opened #138 and #139 with two implementations. #138 keeps logrus around (for now) and #139 just switches over to stdlib's log/slog library. @joelrebel @splaspood @turegano-equinix thoughts?

@mmlb
Copy link
Member Author

mmlb commented Apr 29, 2024

I think I like go-logr/logr's api over log/slog but there's something to be said for being in the stdlib for log/slog. Both apis are pretty similar but I like that logr is explicitly Error and Info levels only and has V() to control which messages actually make it through. I also like that logr has an Error method that takes in an error.

slog has explicit methods for the different levels which is helpful if we use it (we do but only in a couple of files so not that big of a deal imo) but is in the stdlib which is a big +. I don't like that it doesn't have an Error method that requires an error argument though, we definitely use that a bunch. I don't really like the api for tweaking a slog.Logger though it only happens in main and examples.

@mmlb mmlb added the v2 Breaking change appropriate for ironlib v2 label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 Breaking change appropriate for ironlib v2
Projects
None yet
Development

No branches or pull requests

1 participant