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

feat: Impl fmt::Display for canonical Error #580

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

bvrooman
Copy link
Contributor

@bvrooman bvrooman commented Sep 6, 2023

Progress towards upgrading fuel-vm inside fuel-core:

This PR adds fmt::Display to canonical::Error. This is helpful because it allows canonical::Error to be used seamlessly in places where it is replacing std::io::Error. Specific areas include:

    async fn estimate_predicates(
        &self,
        ctx: &Context<'_>,
        tx: HexString,
    ) -> async_graphql::Result<Transaction> {
        let mut tx = FuelTx::from_bytes(&tx.0)?;
        ...

This example requires that the possible error returned by from_bytes(..) implements Display. Without this implementation, the code will fail to compile.

With this change, the upgrade branch compiles locally without error or warning.

@bvrooman bvrooman added the no changelog Skips the CI changelog check label Sep 6, 2023
@bvrooman bvrooman marked this pull request as ready for review September 6, 2023 02:02
@bvrooman bvrooman self-assigned this Sep 6, 2023
@bvrooman bvrooman requested a review from a team September 6, 2023 02:02
@bvrooman bvrooman added this pull request to the merge queue Sep 6, 2023
Merged via the queue into master with commit 7510fb0 Sep 6, 2023
36 of 42 checks passed
@bvrooman bvrooman deleted the bvrooman/feat/impl-display-for-canonical-error branch September 6, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Skips the CI changelog check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants