-
Notifications
You must be signed in to change notification settings - Fork 355
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
PartialEq for errors #253
PartialEq for errors #253
Conversation
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 good and improves a lot of tests.
However, I had some questions on what the canonical format for checking StdError types is and let us just define one pattern and stick to it (I see 2 approached above).
Other that that, just a rebase to fix merge conflicts and this is good to merge.
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 good. Just one idea for further refinement (optional)
c558087
to
f5a671d
Compare
Standardize error args order Add extra matches! / error builders
Merging now, so as to integrate with the other branches. |
Closes #179.