-
Notifications
You must be signed in to change notification settings - Fork 13
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
Implement std::error::Error
for cocoon::Error
#26
Conversation
This adds the `std::error::Error` trait implementation to `cocoon::Error` using the `thiserror` crate. Note that this is only limited to the presence of feature "std".
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.
Thanks for the PR
Co-authored-by: Alexander Fadeev <[email protected]>
Co-authored-by: Alexander Fadeev <[email protected]>
Might be good to update the nightly version in CI |
There is a weird problem with coverage, the coverage works well on that specific nightly version, and it may take a lot of time to fix coverage in case of shifting to another version... Could you change your patch to a classic variant without " |
I've further tested this on my computer and the best I could do is to remove line 36 entirely, but that'd create a very awkward case where people can enable the So maybe we could use |
@madonuko I think it's okay to remove line 36, and maybe someone would use it without std :) BTW, |
I just kind of thought it'd be beneficial to implement |
@madonuko I'm looking whether I can bump the nightly version for coverall... |
Cargo.toml
Outdated
@@ -31,7 +32,7 @@ default = ["std"] | |||
|
|||
# Enables all features, including support of simplified Cocoon API, using `rand::thread_rng`, | |||
# and API related to `std::io`: wrap to writer, unwrap from reader. | |||
std = ["alloc", "rand/std"] | |||
std = ["alloc", "rand/std", "thiserror"] |
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.
@madonuko Let's make it optional, you will enable it in your code.
std = ["alloc", "rand/std", "thiserror"] | |
std = ["alloc", "rand/std"] |
Then I can quickly merge it, I will improve the documentation and I will release it shortly.
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.
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.
Looks pretty much perfect to me
Closed in favor of PR #28 which includes the contents of this PR. |
This adds the
std::error::Error
trait implementation tococoon::Error
using thethiserror
crate.Note that this is only limited to the presence of feature "std".