Itertools::format[_with] docs mention panic within logging macro#941
Itertools::format[_with] docs mention panic within logging macro#941Philippe-Cholet wants to merge 3 commits intorust-itertools:masterfrom
Itertools::format[_with] docs mention panic within logging macro#941Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #941 +/- ##
==========================================
+ Coverage 94.38% 94.45% +0.06%
==========================================
Files 48 49 +1
Lines 6665 7086 +421
==========================================
+ Hits 6291 6693 +402
- Misses 374 393 +19 ☔ View full report in Codecov by Sentry. |
|
@Veetaha Does it seem enough to you? |
Philippe-Cholet
force-pushed
the
format-panic
branch
from
0e90cea to
86b586f
Compare
|
@phimuemue CI of recent #943 was unexpectedly long too (20 minutes more) and I don't know why. |
|
Did we ever think about requiring |
No idea why CI takes so long. I skimmed the individual steps, and they seem to add up to something around 3 minutes. Maybe an infrastructure issue on Github's side? |
|
#307 mentions adding a About CI, I don't know what it is. I looked at the log, saw a link about curl 8.0 (in rust 1.71) having time issues but it does not seem relevant to us (being at rust 1.78). |
src/lib.rs
Outdated
| /// **Panics** if the formatter helper is formatted more than once. | ||
| /// ⚠ This can happen unexpectedly and be hard to debug if used in | ||
| /// _macros of some logging frameworks_ like `tracing`! ⚠ |
There was a problem hiding this comment.
A few requests:
- can you give
**Panics**its own# Header? - can your provide a
should_panicdoctest example of when this panics
(don't addtracingas a dev-dependency though, since it has a different MSRV policy) - can you investigate replacing
⚠with rustdoc's experimental support for first-class warning blocks?
There was a problem hiding this comment.
- I can give it its own header.
- I can simulate a
tracingmacro, and hide it by commenting it out. - About replacing
⚠, we can't use markdown_words_ `tracing`!inside<div class="warning"></div>so either full html<i>words</i> <code>tracing</code>or keep it as is, your call.
There was a problem hiding this comment.
@jswrenn Done. I can dump the second commit if you finally prefer ⚠.
…ng macros I failed to make the (invisible) macro usable as `tracing::info!` with some private module. Renamed it `tracing_info` instead. I add `should_panic` but checked it panics for the right reason.
Markdown can not be used inside it. It renders well in the documentation and in my IDE (Sublime Text: orange text).
Philippe-Cholet
force-pushed
the
format-panic
branch
from
86b586f to
df96c73
Compare
jswrenn
left a comment
jswrenn
left a comment
There was a problem hiding this comment.
A final nit, and then I'm happy to approve!
| /// When the formatter helper is formatted more than once. | ||
| /// | ||
| /// <div class="warning">This can happen unexpectedly and be hard to debug if used in | ||
| /// <i>macros of some logging frameworks</i> like <code>tracing</code>!</div> |
There was a problem hiding this comment.
Instead of <code>tracing</code>, let's link to https://docs.rs/tracing
4 participants
Closes #667 and maybe #307.
I merely update the docs of
Itertools::format[_with], reformulations are welcome: