Skip to content

fix(otel): fix doctests #794

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
merged 2 commits into from
May 8, 2025
Merged

fix(otel): fix doctests #794

merged 2 commits into from
May 8, 2025

Conversation

lcian
Copy link
Member

@lcian lcian commented May 8, 2025

The doctests show usage from the point of view of the sentry crate, as we are importing the subcrate as use sentry::integrations::opentelemetry as sentry_opentelemetry;.
Therefore the first doctest will not compile.
The second will also not compile because we have the imports in the first.
Let's just use ignore on both of them.

@lcian lcian requested a review from Swatinem May 8, 2025 12:57
@lcian lcian enabled auto-merge (squash) May 8, 2025 13:51
Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

you can still run these doctests by pulling in the main sentry crate as a dev dependency.
we use this pattern in the doctests for sentry-core itself.

We are abusing dependency cycles in a very creative way here. Not really a fan of it tbh, but it kinda works.

@lcian lcian changed the title docs(otel): use ignore attribute in doctests fix(otel): fix doctests May 8, 2025
@lcian lcian disabled auto-merge May 8, 2025 15:36
@lcian
Copy link
Member Author

lcian commented May 8, 2025

you can still run these doctests by pulling in the main sentry crate as a dev dependency. we use this pattern in the doctests for sentry-core itself.

We are abusing dependency cycles in a very creative way here. Not really a fan of it tbh, but it kinda works.

Ah cool, we were actually already pulling sentry as a dev dependency for the test feature, didn't think that pulling opentelemetry itself would work :)
I guess cargo is not this dumb :)

@lcian lcian enabled auto-merge (squash) May 8, 2025 15:41
@lcian lcian merged commit 1de559b into master May 8, 2025
14 checks passed
@lcian lcian deleted the lcian/something-1 branch May 8, 2025 15:42
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.

2 participants