Skip to content

Conversation

kevaundray
Copy link
Contributor

@kevaundray kevaundray commented Apr 17, 2025

Similar to #15786

The errors crate pulls in fs-util for an error, so to make it std compatible, it required putting that dependency and the FsError type under a std feature flag

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.

most of the reth-errors = { workspace = true, features = ["std"] } activations should be redundant because enforced via zepter's feature delegation, so I'd like to undo all of those and only enforce where necessary,

likely where we need the fs conversion

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Apr 17, 2025
@kevaundray
Copy link
Contributor Author

most of the reth-errors = { workspace = true, features = ["std"] } activations should be redundant because enforced via zepter's feature delegation, so I'd like to undo all of those and only enforce where necessary,

likely where we need the fs conversion

sorry was not paying attention -- will revert -- I'm not actually sure any crate explicitly uses that From trait, but didn't want to make a breaking change.

We could remove the From trait and manually use RethError::other explicitly in the places that need the conversion (if any), then we can get rid of the fs-util dependency and the std feature flag.

@kevaundray
Copy link
Contributor Author

most of the reth-errors = { workspace = true, features = ["std"] } activations should be redundant because enforced via zepter's feature delegation, so I'd like to undo all of those and only enforce where necessary,
likely where we need the fs conversion

sorry was not paying attention -- will revert -- I'm not actually sure any crate explicitly uses that From trait, but didn't want to make a breaking change.

We could remove the From trait and manually use RethError::other explicitly in the places that need the conversion (if any), then we can get rid of the fs-util dependency and the std feature flag.

Removed the From trait to see if this compiles in ci (my laptop is not great)

@kevaundray kevaundray force-pushed the kw/reth-errors-no-std branch from b6f7004 to 419aabe Compare April 17, 2025 16:51
@kevaundray kevaundray force-pushed the kw/reth-errors-no-std branch from 419aabe to a6473c3 Compare April 17, 2025 16:53
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.

nice, I think this works, let me fix the ci

@mattsse mattsse marked this pull request as ready for review April 17, 2025 17:05
@mattsse mattsse requested a review from gakonst as a code owner April 17, 2025 17:05
@mattsse mattsse enabled auto-merge April 17, 2025 17:06
@mattsse mattsse added this pull request to the merge queue Apr 17, 2025
Merged via the queue into paradigmxyz:main with commit 58c72b7 Apr 17, 2025
43 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Apr 17, 2025
07Vaishnavi-Singh pushed a commit to 07Vaishnavi-Singh/reth that referenced this pull request May 3, 2025
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants