-
Notifications
You must be signed in to change notification settings - Fork 529
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
logrus logging bridge #5356
logrus logging bridge #5356
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5356 +/- ##
=======================================
+ Coverage 62.7% 63.1% +0.3%
=======================================
Files 192 193 +1
Lines 11786 11892 +106
=======================================
+ Hits 7399 7504 +105
- Misses 4170 4171 +1
Partials 217 217
|
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.
It would be good to increase the test coverage.
In my opinion we should also add a test example. It may help not only the users but also other reviewers to understand how it is supposed to be used.
It would be good to add benchmarks, but this would be better added as a separate PR. I do not think it is a must-have given that logrus is not used in performance-critical applications.
e0cd368
to
502f48d
Compare
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Closes #5193.
This adds the logrus log bridge