-
Notifications
You must be signed in to change notification settings - Fork 658
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
attributes: update docs for instrument #2908
base: master
Are you sure you want to change the base?
Conversation
3da25dd
to
1a08d4f
Compare
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.
LGTM otherwise!
tracing-attributes/src/lib.rs
Outdated
/// | ||
/// Note that overlap between the names of fields and (non-skipped) arguments | ||
/// will result in a compile error. | ||
/// will result in recording the value provided to the macro. |
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.
I would add something like "and ignore the value provided to the function." to the sentence as this could be a slightly unpleasant surprise.
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.
Done. I've also changed the paragraph just before because I've noticed it might also be misleading out of context.
300c525
to
da2bd78
Compare
da2bd78
to
17cfc90
Compare
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.
just these nits, but lgtm otherwise!
Motivation
Docs were obsolete for the
instrument
macro.Closes #2895
Solution
I've updated the docs and addeda a doctest example.
This should also be cherry-picked onto
v0.1.x