Skip to content
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

replace failure with thiserror #17

Merged

Conversation

r-arias
Copy link

@r-arias r-arias commented Mar 25, 2020

Consensus seems to be that using failure in libraries is discouraged in
in favor of using the std Error trait.

thiserror provides the failure features used in tokio-socks while
creating an error conforming to the std Error trait.

It requires a minimum rustc version of 1.31, but the same is true for
failure, so this should not be trouble.

For people relying on the fact that tokio_socks::Error is Fail this would probably be a breaking change, but I am not sure it would be hard to fix.

Consensus seems to be that using failure in libraries is discouraged in
in favor of using the std Error trait.

thiserror provides the failure features used in tokio-socks while
creating an error conforming to the std Error trait.

It requires a minimum rustc version of 1.31, but the same is true for
failure, so this should not be trouble.
Copy link
Owner

@sticnarf sticnarf left a comment

Choose a reason for hiding this comment

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

Looks great.

Actually the Error still implements Fail because there's impl<E: StdError + Send + Sync + 'static> Fail for E. I believe it won't make trouble for anyone.

@sticnarf sticnarf merged commit 6b3b305 into sticnarf:master Mar 27, 2020
@r-arias r-arias deleted the feature/replace-failure-with-thiserror branch March 27, 2020 18:25
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