Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Use Error::description only for rust below 1.42 #285

Conversation

AnderEnder
Copy link
Contributor

Error::description has been documented as soft-deprecated since 1.27.0 (17 months ago). It is going to be hard-deprecated soon (1.42)

This PR:

  • adds configuration options: has_error_description_deprecated
  • implements Error::description conditionally
  • create a conditional macro impl_error_chain_kind without usage of Error::description

Related PR: rust-lang/rust#66919
Alternative PR(with removed description): #283

Copy link
Contributor

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

From your three PRs, I think this is the best solution and what we should do! I only left one inline comment.

src/error_chain.rs Outdated Show resolved Hide resolved
@AnderEnder AnderEnder force-pushed the conditional-macro-error-description branch from fd24f31 to 6ff71d8 Compare January 3, 2020 23:41
@AndyGauge
Copy link
Contributor

I'm a fan of this approach.

Comment on lines +79 to +84
#[doc(hidden)]
#[cfg(not(has_error_description_deprecated))]
#[macro_export(local_inner_macros)]
macro_rules! call_to_deprecated_description {
($e:ident) => { ::std::error::Error::description($e) };
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this need a doc comment, too? Wouldn't #![deny(missing_documentation)] trigger in case not(has_error_description_deprecated)?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's why CI fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested, it fails and reverted it back

@AnderEnder AnderEnder force-pushed the conditional-macro-error-description branch from 6ff71d8 to 11dd09a Compare January 4, 2020 18:57
@AnderEnder AnderEnder force-pushed the conditional-macro-error-description branch from 11dd09a to c1018b6 Compare January 5, 2020 18:15
@AndyGauge AndyGauge merged commit ea81711 into rust-lang-deprecated:master Jan 14, 2020
@ndmitchell
Copy link

Any chance of a release containing this fix? Thanks

@AndyGauge
Copy link
Contributor

Yeah, give me a week or two, but I will release it.

@LukasKalbertodt
Copy link
Contributor

@AndyGauge Ping regarding release. People start noticing this on stable (#286). Thanks! :)
(I think I don't have the rights to release this...)

@AndyGauge
Copy link
Contributor

shoot you are right, I'm behind on this. Tomorrow.

@AndyGauge
Copy link
Contributor

🎉 0.12.2 is on crates.io Let me know if it breaks production 😉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants