-
Notifications
You must be signed in to change notification settings - Fork 847
mock: complete API documentation including expect module #2494
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
Merged
Conversation
This file contains hidden or 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.
23cc2cb to
da9f9fc
Compare
da9f9fc to
c467577
Compare
0xPoe
approved these changes
Oct 31, 2024
Contributor
0xPoe
left a comment
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
expectmodule, which contains constructor functions for many ofthe other
tracing-mockmodules needs documentation and examples.This change adds documentation to the
expectmodule and all the publicAPIs within it. This includes doctests on all the methods which serve as
examples.
Solution
The lint for
missing_docshas been enabled for the entiretracing-mockcrate! 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::msgconstructor could beoverridden by a subsequent usage of
with_fields. The shorthandexpect::message()was renamed toexpect::msgto 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
ExpectedSpanis required.This change also sets the
missing_docslint to warn for the entiretracing-mockcrate, making it ready to publish (once backported).Refs: #539