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

Replace anyhow with thiserror #1093

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Levi0804
Copy link

@Levi0804 Levi0804 commented Feb 23, 2025

This PR is a work in progress addressing #916.

I am aware that some of the changes in this PR introduce breaking changes in other parts of the workspace. I created this PR to take reviews on whether my approach looks good before proceeding further. Please let me know if these changes look good to you.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

smol style nit

Comment on lines +22 to +30
#[derive(Debug, thiserror::Error)]
enum PreimageServerError {
#[error("Failed to serve preimage request: {0}")]
PreimageRequestFailed(PreimageOracleError),
#[error("Failed to route hint: {0}")]
RouteHintFailed(PreimageOracleError),
#[error("Join error: {0}")]
ExecutionError(#[from] tokio::task::JoinError),
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is used it pub api and should also be pub

a one-line doc also wouldn't hurt here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with @mattsse here

Copy link
Author

Choose a reason for hiding this comment

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

Alright, I'll make these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

3 participants