Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mladedav
Copy link
Contributor

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

Copy link
Contributor

@kaffarell kaffarell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise!

///
/// 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.
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@mladedav mladedav force-pushed the dm/instrument-docs branch 2 times, most recently from 300c525 to da2bd78 Compare May 2, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: tracing field names CAN actually have a dot in them now
2 participants