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

use 'thiserror' to define 'Error' #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danieleades
Copy link
Contributor

actually doesn't add a great deal of value to such a simple error type, but leaving here for discussion

@VirxEC
Copy link
Owner

VirxEC commented Oct 20, 2024

Does thiserror actually implement anything that wasn't before? I don't want to add an additional dependency for something so minor.

@danieleades
Copy link
Contributor Author

Does thiserror actually implement anything that wasn't before? I don't want to add an additional dependency for something so minor.

no it doesn't add anything that wasn't there before. It's a compile-time only macro- it's entirely equivalent at runtime.

@VirxEC
Copy link
Owner

VirxEC commented Oct 20, 2024

I don't see the point, then. I love thiserror and I've used it before, but I think it's unnecessary here. The reason why use std::error::Error; is the only import from std is that this crate can go no-std once Rust 1.81 becomes a little older and it can be changed to importing from core instead. All this crate does is a bit of math, seems like a shame that isn't not no-std (with an std feature.)

@danieleades
Copy link
Contributor Author

No-std support possibly coming soon: dtolnay/thiserror#304

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