Skip to content

Comments

Add error type wrapping all possible errors#122

Draft
DevinR528 wants to merge 2 commits intoheadcrab-rs:masterfrom
DevinR528:error-type
Draft

Add error type wrapping all possible errors#122
DevinR528 wants to merge 2 commits intoheadcrab-rs:masterfrom
DevinR528:error-type

Conversation

@DevinR528
Copy link

Just wondering if I'm moving in the right direction? Any naming suggestions/ideas would be helpful as I am still getting my head around everything.

use thiserror::Error;

#[derive(Error, Debug)]
pub enum CrabError {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a separate error type for target and one for symbol.

Copy link
Author

@DevinR528 DevinR528 Oct 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does there still need to be a top-level wrapper for pub type CrabResult<T> = Result<T, ErrorType>;? Something like

pub enum CrabError {
    Target(crate::target::TargetError),
    Symbol(crate::symbol::SymbolError),
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should each mod (target and symbol) have their own Result type?

Copy link
Contributor

@bjorn3 bjorn3 Oct 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does there still need to be a top-level wrapper for pub type CrabResult = Result<T, ErrorType>;? Something like

That could be useful.

Or should each mod (target and symbol) have their own Result type?

Yes please.

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