- remove default trace level and make log lvl mandatory on span! macro #1025
Conversation
jxs
force-pushed
the
update-trace-macros
branch
3 times, most recently
from
89ffd06 to
0e92f19
Compare
- add trace_span!, debug_span!, info_span!, warn_span! and error_span! macros that behave as span! macro, but with defined log levels
|
ok updated it! |
There was a problem hiding this comment.
This looks good to me!
I think that there are potentially some places where it might be good to the new macros in examples or tests rather than using span! with a level, just so that users see the new macros in use in more cases. But, I'm willing to merge this without making that change.
| error_span!("foo", bar = 2, baz = 3); | ||
| error_span!("foo", bar = 2, baz = 4,); | ||
| error_span!("bar"); | ||
| error_span!("bar",); |
There was a problem hiding this comment.
Since these tests have gotten so repetitive, part of me wonders if we should be generating them with a macro. But since the purpose of these tests is to catch any macro compilation errors, that's probably a mistake...
I can make another PR with those changes, no problem, do you want to open an issue to track those cases? |
|
@jxs sure! Also, another follow-up is that it would be great if after this PR merges, we update the crates in the |
|
yes sure! i am going to clone the repo and check which ones fail to build |
…macro (#1025) ## Motivation Was determined that having the span! macro default to the TRACE level is probably not ideal (see discussion on #952). Closes #1013 ## Solution Remove default trace level and make log lvl mandatory on span! macro, and add the respective `trace_span!`, `debug_span!`, `info_span!`, `warn_span!` and `error_span!` macros that behave as span! macro, but with defined log levels ## Notes I think this is it, also removed some captures that were repeated, and some testcases that also seemed repeated after adding the mandatory log level, but please review it, if more tests or examples are needed happy to provide (tried to find a way to get the generated macros log level, but didn't find one, if there is a way i can add tests to assert that the generated macro has the matching log level ). thanks
Motivation
Was determined that having the span! macro default to the TRACE level is probably not ideal (see discussion on #952).
closes #1013
Solution
remove default trace level and make log lvl mandatory on span! macro, and add the respective
trace_span!,debug_span!,info_span!,warn_span!anderror_span!macros that behave as span! macro, but with defined log levelsNotes
I think this is it, also removed some captures that were repeated, and some testcases that also seemed repeated after adding the mandatory log level, but please review it, if more tests or examples are needed happy to provide (tried to find a way to get the generated macros log level, but didn't find one, if there is a way i can add tests to assert that the generated macro has the matching log level ).
thanks