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 instead of derive_more for error handling #555

Merged
merged 2 commits into from
Feb 14, 2022
Merged

use thiserror instead of derive_more for error handling #555

merged 2 commits into from
Feb 14, 2022

Conversation

koushiro
Copy link
Collaborator

Signed-off-by: koushiro koushiro.cqx@gmail.com

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@sorpaas
Copy link
Member

sorpaas commented Jan 19, 2022

Can you briefly explain why? I'm not following the comparison between thiserror and derive_more, and this PR doesn't look like it saved much LOC either.

@koushiro
Copy link
Collaborator Author

derive_more is a good toolkit that provides derive macros of many standard library traits, but thiserror is more focused on error handling (although the usage here is not much different), and as far as I know, thiserror is the most commonly used error handling library in the current Rust community.

@koushiro
Copy link
Collaborator Author

@sorpaas what do you think?

@koushiro
Copy link
Collaborator Author

Since paritytech/substrate#10696 has been merged, I think this PR should be fine, what do you think? @sorpaas

@sorpaas sorpaas merged commit edde100 into polkadot-evm:master Feb 14, 2022
@koushiro koushiro deleted the use-thiserror branch February 15, 2022 01:21
abhijeetbhagat pushed a commit to web3labs/frontier that referenced this pull request Jan 11, 2023
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