-
Notifications
You must be signed in to change notification settings - Fork 224
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
Refactor errors #158
Refactor errors #158
Conversation
- use thiserror and anomaly - rename ValidationError -> validate::Error - rename ValidationErrorKind -> Kind
- we do not use anomalys serializer feature cleanup Cargo.toml: remove failure and unused feature: - we do not use anomalys serializer feature - remove last place which used failure (looks like we accidentally imported `failure::_core::fmt::Debug` instead of just `std::fmt::Debug`
codecov wasn't reporting because of a missing report for the base commit. Deleted its comment. |
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.
Tis extra dope!
Generally I'd err on the side of more qualification so use crate::error
and then error::Error
and error::Kind
. Also left some remarks inline. ✌️
Codecov Report
@@ Coverage Diff @@
## master #158 +/- ##
=========================================
+ Coverage 37.42% 37.5% +0.07%
=========================================
Files 91 91
Lines 3140 3144 +4
Branches 484 485 +1
=========================================
+ Hits 1175 1179 +4
- Misses 1690 1691 +1
+ Partials 275 274 -1
Continue to review full report at Codecov.
|
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.
Generally I'd err on the side of more qualification so
use crate::error
and thenerror::Error
anderror::Kind
.
@liamsi Think this could be addressed in this PR.
Otherwise: 🤘 🎥 🎊 🗞
Merging as is now. Will address below in a separate PR which will add an ADR, too.
|
Consistent error handling across the repo.
I have written up some notes briefly explaining what "consistent" means here.
Will post themPosted in #150 (comment). They can be used for an ADR if that's desired.closes #150