-
Notifications
You must be signed in to change notification settings - Fork 742
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: complete API documentation including expect module #2494
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 adds documentation to the collector module itself and to all the public APIs in the module. This includes doctests on all the methods that serve as examples. Additionally the implementation for the `Expect` struct has been moved into the module with the definition, this was missed in #2369. Refs: #539
It creates too many issues (need to add `allow_deprecated` annotations all over the place) and they will actually make fixing those tests to use the new (but as yet unimplemented) `MockCollector::close_span` more difficult.
They are! I removed the referce to `MockCollector::close_span` as it doesn't yet exist.
Also cleaned up the text and tried to apply the code review comments from #2426 to what I'd written already.
Co-authored-by: Eliza Weisman <[email protected]>
All based on the lovely code review feedback I've been receiving.
… into hds/mock-collector-docs
This change adds documentation to the span module and all the public APIs within it. This includes doctests on all the methods which serve as examples. Additionally, the validation on `ExpectedSpan` was improved so that it validates the level and target during `enter` and `exit` as well as on `new_span`. The method `ExpectedSpan::with_field` was renamed to `with_fields` (plural) to match the same method on `ExpectedEvent` (and because multiple fields can be passed to it). A copy-paste typo was also fixed in the documentation for `ExpectedEvent::with_contextual_parent`. Refs: #539
This change adds documentation to the `field` module and all the public APIs within it. This includes doctests on all the methods which serve as examples. Additionally, the `field::msg` function (which constructs a field with name "message" and the provided value) was moved to `expect::message`. This is part of a unification of all expectation constructors inside the `expect` module. Refs: #539
The PR documenting the subscriber module.
Was previously `field::msg`.
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. The `expect` module, which contains constructor functions for many of the other `tracing-mock` modules needs documentation and examples. This change adds documentation to the `expect` module and all the public APIs within it. This includes doctests on all the methods which serve as examples. The lint for `missing_docs` has been enabled for the entire `tracing-mock` crate! Further lints which are common in other crates in this project haven't been enabled. This will be done in a later PR. The `event::msg("message")` constructor was removed, in favor of requiring an explicit construction via `expect::event().with_fields(expect::msg("message"))`. This is appropriate to reduce the API surface that would need to be supported in the future and also because the `event::msg` constructor could be overridden by a subsequent usage of `with_fields`. The `span::named("name")` constructor was removed, in favor of requiring an explicit construction via `expect::span.with_name("name")`. The latter isn't much longer.
hds
force-pushed
the
hds/mock-docs-expect
branch
from
October 30, 2024 17:00
23cc2cb
to
da9f9fc
Compare
hds
force-pushed
the
hds/mock-docs-expect
branch
from
October 30, 2024 17:07
da9f9fc
to
c467577
Compare
Rustin170506
approved these changes
Oct 31, 2024
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.
Looks good to me. Thanks! 👍
davidbarsky
approved these changes
Nov 1, 2024
hds
added a commit
that referenced
this pull request
Nov 11, 2024
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. The `expect` module, which contains constructor functions for many of the other `tracing-mock` modules needs documentation and examples. This change adds documentation to the `expect` module and all the public APIs within it. This includes doctests on all the methods which serve as examples. The lint for `missing_docs` has been enabled for the entire `tracing-mock` crate! This has been done together with all the other lints that are enabled on the other crates in this project. The `event::msg("message")` constructor was removed, in favor of requiring an explicit construction via `expect::event().with_fields(expect::msg("message"))`. This is appropriate to reduce the API surface that would need to be supported in the future and also because the `event::msg` constructor could be overridden by a subsequent usage of `with_fields`. The shorthand `expect::message()` was renamed to `expect::msg` to make this change less burdensome. The `span::named("name")` constructor was removed, in favor of requiring an explicit construction via `expect::span.with_name("name")`. The latter isn't much longer and since #3097, a string with the name can be passed directly everywhere that an `ExpectedSpan` is required. This change also sets the `missing_docs` lint to warn for the entire `tracing-mock` crate, making it ready to publish (once backported). Refs: #539
This was referenced Nov 18, 2024
hds
added a commit
that referenced
this pull request
Nov 20, 2024
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. The `expect` module, which contains constructor functions for many of the other `tracing-mock` modules needs documentation and examples. This change adds documentation to the `expect` module and all the public APIs within it. This includes doctests on all the methods which serve as examples. The lint for `missing_docs` has been enabled for the entire `tracing-mock` crate! This has been done together with all the other lints that are enabled on the other crates in this project. The `event::msg("message")` constructor was removed, in favor of requiring an explicit construction via `expect::event().with_fields(expect::msg("message"))`. This is appropriate to reduce the API surface that would need to be supported in the future and also because the `event::msg` constructor could be overridden by a subsequent usage of `with_fields`. The shorthand `expect::message()` was renamed to `expect::msg` to make this change less burdensome. The `span::named("name")` constructor was removed, in favor of requiring an explicit construction via `expect::span.with_name("name")`. The latter isn't much longer and since #3097, a string with the name can be passed directly everywhere that an `ExpectedSpan` is required. This change also sets the `missing_docs` lint to warn for the entire `tracing-mock` crate, making it ready to publish (once backported). Refs: #539
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The
expect
module, which contains constructor functions for many ofthe other
tracing-mock
modules needs documentation and examples.This change adds documentation to the
expect
module and all the publicAPIs within it. This includes doctests on all the methods which serve as
examples.
Solution
The lint for
missing_docs
has been enabled for the entiretracing-mock
crate! This has been done together with all theother lints that are enabled on the other crates in this project.
The
event::msg("message")
constructor was removed, in favor ofrequiring an explicit construction via
expect::event().with_fields(expect::msg("message"))
. This isappropriate to reduce the API surface that would need to be supported in
the future and also because the
event::msg
constructor could beoverridden by a subsequent usage of
with_fields
. The shorthandexpect::message()
was renamed toexpect::msg
to make thischange less burdensome.
The
span::named("name")
constructor was removed, in favor of requiringan explicit construction via
expect::span.with_name("name")
. Thelatter isn't much longer and since #3097, a string with the name can
be passed directly everywhere that an
ExpectedSpan
is required.This change also sets the
missing_docs
lint to warn for the entiretracing-mock
crate, making it ready to publish (once backported).Refs: #539