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
zapcore: add warning as Level #1429
base: master
Are you sure you want to change the base?
Conversation
Some packages set their value for WarnLevel to "warning". For easier integration add "warning" as accepted WarnLevel. Example for a package that is using "warning": https://github.com/sirupsen/logrus/blob/dd1b4c2e81afc5c255f216a722b012ed26be57df/logrus.go#L68 Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Friendly ping for feedback to @abhinav or @JacobOaks . |
Hey @florianl, I think this change incurs some technical debt for a small amount of flexibility. The current level MarshalText logic is simplified because there is either a capitalized or lower case representation of the each level, where the level is represented in a consistent tense. From my perspective, the special check case in level_test.go makes the level test code a bit more difficult to main. |
Hey @r-hang thanks for the feedback! There are cases where this packages is expected to handle input, that is generated from other packages, that are using the same levels, but slightly different level text - |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1429 +/- ##
=======================================
Coverage 98.69% 98.69%
=======================================
Files 53 53
Lines 2997 2997
=======================================
Hits 2958 2958
Misses 31 31
Partials 8 8 ☔ View full report in Codecov by Sentry. |
@r-hang what do you think about having a dedicated test for unmarshaling |
Some packages set their value for WarnLevel to "warning". For easier integration add "warning" as accepted WarnLevel.
Example for a package that is using "warning":
https://github.com/sirupsen/logrus/blob/dd1b4c2e81afc5c255f216a722b012ed26be57df/logrus.go#L68