Skip to content

docs: Document integrations and the Hub better #291

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 4 commits into from
Nov 19, 2020

Conversation

flub
Copy link
Contributor

@flub flub commented Nov 18, 2020

This adds more documentation for how to use integrations and uses much
more intra-doc links. The Hub documentation as well as the anyhow
integration documentation have also seen various improvements.

The docs no build again without any warnings.

#skip-changelog

This adds more documentation for how to use integrations and uses much
more intra-doc links.  The Hub documentation as well as the anyhow
integration documentation have also seen various improvements.
//! This integration adds a new event *source*, which allows you to create events directly
//! from an [`anyhow::Error`] struct. As it is only an event source it only needs to be
//! enabled using the `anyhow` cargo feature, it does not need to be enabled in the call to
//! [`sentry::init`](../../fn.init.html).
Copy link
Member

Choose a reason for hiding this comment

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

I guess these links are made in a way that they work when you are building sentry docs, and you expose this via integrations::anyhow. However you should still be able to build the docs for this individually. Maybe intra-doc-links will just work when you have sentry as a (circular) dev-dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ugh, this was the only solution I could get this to work reasonably. I didn't think of it being standalone. I guess that's how it ends up on docs.rs though?

I tried adding the circular dependency under the "doc" feature but cargo was not having any of that. Do you think as a dev dependency would have more chance of working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, sentry was already a circular dev dependency. so that doesn't work. So it's basically between this option which break the doc.rs build for the standalone crate, or point it to https://docs.rs/sentry/*/sentry/fn.init.html I think. Maybe the latter is the least evil :(

@@ -13,6 +26,8 @@
//! capture_anyhow(&err);
//! }
//! ```
//!
//! [`anyhow::Error`]: https://docs.rs/anyhow/*/anyhow/struct.Error.html
Copy link
Member

Choose a reason for hiding this comment

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

I think intra-doc-links should handle this?

Which reminds me, the relative links from above relative links will also fail when this is converted to the README via cargo-readme :-(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what happened here. When trying to use the plain intra-doc links cargo +nightly doc did not give any warning, suggesting it was happy with the link, but they did not work, they just rendered literally. I have no idea what I was doing wrong and failed to find. This was the only way I got this to link correctly, it's not very nice but at least it's not wrong.

If you can get it to work with the intra-doc link then let me know what I did wrong!

@@ -331,11 +344,11 @@ impl Hub {

/// End the current Release Health Session.
///
/// See the global [`end_session`](crate::end_session_with)
/// for more documentation.
/// See the global [`sentry::end_session`](crate::end_session) for more documentation.
Copy link
Member

Choose a reason for hiding this comment

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

👍 thanks for fixing this ;-)

@Swatinem
Copy link
Member

error[E0432]: unresolved import `sentry::integrations::debug_images`
 --> src/lib.rs:119:27
  |
5 | use sentry::integrations::debug_images::DebugImagesIntegration;
  |                           ^^^^^^^^^^^^ could not find `debug_images` in `integrations`

This fails when doing a no-default-features build. I think it should be possible to either add a self-reference with all-features to the dev-dependencies, or add the sentry-debug-images to the dev-dependencies and reference it directly. You decide. But if you want to go with the self-reference, can you also update all examples to use the sentry::integrations module?

Suggestions from review, add_integration was the most important.

Co-authored-by: Arpad Borsos <arpad.borsos@sentry.io>
@flub
Copy link
Contributor Author

flub commented Nov 19, 2020

error[E0432]: unresolved import `sentry::integrations::debug_images`
 --> src/lib.rs:119:27
  |
5 | use sentry::integrations::debug_images::DebugImagesIntegration;
  |                           ^^^^^^^^^^^^ could not find `debug_images` in `integrations`

This fails when doing a no-default-features build. I think it should be possible to either add a self-reference with all-features to the dev-dependencies, or add the sentry-debug-images to the dev-dependencies and reference it directly. You decide. But if you want to go with the self-reference, can you also update all examples to use the sentry::integrations module?

Or we could change the feature gates to #[cfg(any(doc, feature = "debug_images"))]? I think this is what they generally seems to recommend?
https://doc.rust-lang.org/rustdoc/advanced-features.html

@Swatinem
Copy link
Member

Or we could change the feature gates to #[cfg(any(doc, feature = "debug_images"))]?

I don’t think that will work, as you can’t pull in the needed dependency.
Or you can just disable the doctest via:

/// # #[cfg(feature = "debug-images")] {
/// …
/// # }

Floris Bruynooghe added 2 commits November 19, 2020 13:26
We this will get tested in some other configuration that does have the
feature, it's fine.
This is not as nice as now they point to docs.rs, but it seems
impossible to otherwise point to the sentry crate with intra-doc links
as we'd need circular dependencies for the doc feature, which cargo
doesn't allow.
@Swatinem Swatinem merged commit fc64df8 into master Nov 19, 2020
@Swatinem Swatinem deleted the docs/integrations-hub branch November 19, 2020 12:56
jan-auer added a commit that referenced this pull request Nov 25, 2020
* master: (59 commits)
  fix: Correctly apply environment from env (#293)
  fix: Make Rust 1.48 clippy happy (#294)
  docs: Document integrations and the Hub better (#291)
  ref: Remove deprecated error-chain and failure crates (#290)
  release: 0.21.0
  meta: Update Changelog
  feat: End sessions with explicit status (#289)
  fix: Scope transaction name can be overriden in sentry-actix (#287)
  fix: sentry-actix should not capture client errors (#286)
  fix: Clean up sentry-actix toml (#285)
  ref: Remove empty integrations (#283)
  feat: Add support for actix-web 3 (#282)
  feat: Preliminary work to integrate Performance Monitoring (#276)
  ref: Introduce a SessionFlusher (#279)
  fix: Set a default environment based on debug_assertions (#280)
  ref: Rearchitect the log and slog Integrations (#268)
  ref: Deprecate public fields on Integrations (#267)
  ci: Make testfast actually fast (#273)
  fix: Update surf and unbreak CI (#274)
  ci: Use smarter cache action (#272)
  ...
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