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

mock: add README to tracing-mock #2362

Merged
merged 23 commits into from Nov 15, 2022
Merged

mock: add README to tracing-mock #2362

merged 23 commits into from Nov 15, 2022

Conversation

hds
Copy link
Contributor

@hds hds commented Nov 2, 2022

Motivation

There has been interest around publishing tracing-mock to crates.io for some time. In order to make this possible, documentation and some code clean up is needed.

Specifically I want to have access to tracing-mock within parts of tokio to write tests that ensure that the correct calling location is picked up for all spawn* functions when the tracing feature is enabled.

Solution

This change starts that process by adding a README for tracing-mock. The README follows the standard format for all tracing crates and includes 2 examples. Additionally an integration test has been added for each example.

The README describes steps when using the tracing 1.0 from crates.io and tracing-mock from the v0.1.x branch. The tests target the current branch.

There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.

This change starts that process by adding a README for `tracing-mock`.
The README follows the standard format for all `tracing` crates and
includes 2 examples. Additionally an integration test has been added for
each example.

The README describes steps when using the `tracing` 1.0 from crates.io
and `tracing-mock` from the `v0.1.x` branch. The tests target the
current branch.

Refs: #539
@hds hds requested review from hawkw, davidbarsky and a team as code owners November 2, 2022 14:40
@hds hds changed the title mock: add README to tracing-mock mock: add README to tracing-mock v0.1.x Nov 2, 2022
@hds hds changed the title mock: add README to tracing-mock v0.1.x mock: add README to tracing-mock Nov 2, 2022
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Show resolved Hide resolved
tracing-mock/README.md Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
hds and others added 9 commits November 3, 2022 17:17
tracing-mock/tests/readme.rs Outdated Show resolved Hide resolved
The readme.rs test file has been removed, since the tests from the
README for `tracing-mock` will get run as part of the doc test.

Necessarily, the examples in the README now target the master branch,
and not the `v0.1.x` branch.
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Overall, this looks great. The one real blocker is that the docs build is currently failing due to the relative link to the LICENSE file, which is no longer valid when the README is included in the RustDoc. If we make that an absolute link, I'll be happy to merge this!

I also commented on a few other nits, but they're not blockers.

tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
tracing-mock/README.md Outdated Show resolved Hide resolved
hds and others added 5 commits November 8, 2022 09:26
Suggestion from @hawkw

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
- Fixed the link to the license (so it's absolute)
- Link to tracing docs when describing what tracing-mock does
- Fixed the test crate version example
- Removed reference to removed test file (no longer needed)
@hawkw
Copy link
Member

hawkw commented Nov 11, 2022

now that #2377 has merged, the examples here will need to be updated to match the new APIs.

@hawkw
Copy link
Member

hawkw commented Nov 11, 2022

~~looks like the most recent docs build CI failures are due to an issue with our netlify config, rather than something in the new docs: https://app.netlify.com/sites/tracing-rs/deploys/636a9cac2ea34100084c2ad4

i'm gonna try to get that fixed~~

edit: actually, never mind, looks like there's link breakage as well: https://app.netlify.com/sites/tracing-rs/deploys/636e8c854b113a0008951581#L249

Comment on lines 39 to 42
[`Collector`](https://tracing-rs.netlify.app/tracing/#collectors)
that allows asserting on the order and contents of
[spans]https://tracing-rs.netlify.app/tracing/#spans) and
[events](https://tracing-rs.netlify.app/tracing/#events).
Copy link
Member

Choose a reason for hiding this comment

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

rather than linking to the netlify master docs here, i think we should probably be linking to the release docs on docs.rs, at least in the release version of these docs...we can change them to point to docs.rs when we do the v0.1.x backport, if you think having this version of the documentation point at the v0.2.x documentation for tracing, but it's a thing we'll need to remember...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My idea was to link to both the docs.rs docs and the master docs once we publish on crates.io, as that's what is done in the other crates in this repo. Since we don't have the docs.rs ones yet, I left it out entirely.

tracing-mock/README.md Outdated Show resolved Hide resolved
hds and others added 3 commits November 15, 2022 10:14
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Also changed `.done()` to `.only()` and remove `.close_span()` entirely
because it's not actually implemented yet.
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks great to me! thank you!

@hawkw hawkw enabled auto-merge (squash) November 15, 2022 22:17
@hawkw hawkw merged commit db84302 into master Nov 15, 2022
@hawkw hawkw deleted the hds/tracing-mock-readme branch November 15, 2022 22:18
davidbarsky added a commit that referenced this pull request Sep 27, 2023
There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.

Specifically **I** want to have access to `tracing-mock` within parts of
`tokio` to write tests that ensure that the correct calling location is
picked up for all `spawn*` functions when the `tracing` feature is
enabled.

This change starts that process by adding a README for `tracing-mock`.
The README follows the standard format for all `tracing` crates and
includes 2 examples. The README is included in the `lib.rs` docs using
`#[doc = include_str!(...)]`, so that the same documentation is included
in the crate, and the examples are tested when running doctests.

The README describes steps when using the `tracing` 1.0 from crates.io
and `tracing-mock` from the `v0.1.x` branch. The tests target the
current branch.

Co-authored-by: David Barsky <me@davidbarsky.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Oct 1, 2023
There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.

Specifically **I** want to have access to `tracing-mock` within parts of
`tokio` to write tests that ensure that the correct calling location is
picked up for all `spawn*` functions when the `tracing` feature is
enabled.

This change starts that process by adding a README for `tracing-mock`.
The README follows the standard format for all `tracing` crates and
includes 2 examples. The README is included in the `lib.rs` docs using
`#[doc = include_str!(...)]`, so that the same documentation is included
in the crate, and the examples are tested when running doctests.

The README describes steps when using the `tracing` 1.0 from crates.io
and `tracing-mock` from the `v0.1.x` branch. The tests target the
current branch.

Co-authored-by: David Barsky <me@davidbarsky.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
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.

None yet

3 participants