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

Move from failure to thiserror #12

Closed
wants to merge 1 commit into from

Conversation

ZeroErrors
Copy link

Changes

  • Moved from failure to thiserror for error handling
  • Bumped version to 0.5.0 due to API breakage

Description

As of failure 0.1.8 rust-lang-nursery/failure#347 it has been deprecated.

Additionally when interacting with this library you must either use failure or map the errors from failure::Error to std::error::Error via failure::Error::compat(). By instead using thiserror to provide a custom error type not only can we provide better information to callers but also means errors use std::error::Error and are easier to interact with.

bors bot added a commit to rust-embedded/cargo-binutils that referenced this pull request May 20, 2020
73: Move from failure to anyhow r=therealprof a=ZeroErrors

## Changes
- Move from `failure` to `anyhow` for error handling

## Description
As of `failure 0.1.8` it has been deprecated ([rust-lang-deprecated/failure#347](rust-lang-deprecated/failure#347)) in favor of using standard rust errors and it is recommended to do that via libraries such as [`anyhow`](https://github.com/dtolnay/anyhow) or [`thiserror`](https://github.com/dtolnay/thiserror).

I chose to go with `anyhow` as its the simplest to convert over to and also since `cargo-binutils` isn't expected to be used as a library I see no need for more detailed error types (See: [dtolnay/anyhow#comparison-to-thiserror](https://github.com/dtolnay/anyhow#comparison-to-thiserror)).

---

**Note:** `failure` isn't fully removed yet as the `rustc-cfg` dependency uses `failure`. I have opened a PR on that repo to hopefully get that changed (japaric/rustc-cfg#12)

r? @therealprof

Co-authored-by: Zero <nick@zero.dev>
@japaric
Copy link
Owner

japaric commented Jul 6, 2022

thanks for the PR. I already merged #15

@japaric japaric closed this Jul 6, 2022
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