-
Notifications
You must be signed in to change notification settings - Fork 19
Chores: Edition 2018, quick-error -> thiserror, deps update - #15
Conversation
Required by error frameworks such as `thiserror`, `anyhow`, and `eyre`.
drahnr
commented
Nov 27, 2020
Hey @tailhook, is there anything left to do here?
drahnr
commented
Dec 19, 2020
☃️ it'd be awesome if I could get some feedback on this 🙂
tailhook
commented
Dec 22, 2020
Yes, sorry. This has so much stuff, I don't have enough time to review. I'm also not sure if I agree with thiserror and fs_err. This crate has their own errors to have both, nice error messages or performance on failure path if needed.
But I still need to look at details, which I had no time to do yet. Hopefully, I'll find some time to take a look soon.
drahnr
commented
Jan 21, 2021
thiserror provides a zero overhead and zero extra weight, it's just convenience for error definition. fs_err adds path information to std::io::Error messages and does not introduce an extra error type iirc. So both are effectively no interface changes.
The rest is mostly chore.
The thing that is breaking, are the additional bounds on Box<dyn Explainable> which is now Box<dyn Explainable + Send + Sync + 'static, but I am not 100% sure about that change being needed, it was needed for the use case though I had, but that resulted in this rustc issue rust-lang/rust#78700
drahnr
commented
Feb 20, 2021
Anything I can add or do here?
A bunch of chores, should be fairly well separated into individual commits.