-
Notifications
You must be signed in to change notification settings - Fork 81
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
Use provide_any
feature in error-stack
#697
Conversation
d46d0a5
to
afba1bd
Compare
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.
Can we generally just do a quick ctrl + f
for all occurrences of provider
and make sure they're not disclaimers about stuff we've included, etc. (Did we mention it in the README for example)
Then happy to approve
//! [`attach`]: Report::attach | ||
//! [`error_stack::provider`]: crate::provider | ||
//! [`Provider` API]: https://rust-lang.github.io/rfcs/3192-dyno.html |
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.
Is this still needed?
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.
This was only used in the above comment. If it would have been needed, the docs-test would fail.
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.
I'm on about the the [Provider
API] link, not the removed line for error_stack::provider
. I don't think docs-tests would fail if you add an unused link?
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.
Oh, sure. Yes, this is already used in the first paragraph of the section:
//! This crate uses the [`Provider` API] to provide arbitrary data. This can be done either by
//! [`attach`]ing them to a [`Report`] or by providing it directly when implementing [`Context`].
//! The blanket implementation of [`Context`] for [`Error`] will provide the [`Backtrace`] to be
//! requested later.
No, it won't complain if you add unused links (sadly) 😢
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.
Gotcha, GitHub wasn't letting me see the above code even when I clicked on the expand button :(
Already did that when updating the docs
You already have approved. |
I changed it to comment when I clicked submit Review but GitHub decided to ignore me |
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.
🎉
🌟 What is the purpose of this PR?
When
error-stack
was released, theprovide_any
feature has not landed on nightly yet.🔗 Related links
🚫 Blocked by
nightly-2022-06-26
#696🔍 What does this change?
provider
and usecore::any
instead📜 Does this require a change to the docs?
core::any
instead