-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
standalone rpc_types #4088
standalone rpc_types #4088
Conversation
Will now figure out how to delete the duplicate code from https://github.com/paradigmxyz/reth/blob/c423514321c2d2e67a156681d5602cb1707852e3/crates/rpc/rpc-types/src/eth/transaction/mod.rs |
Codecov Report
... and 391 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps there was a misunderstanding,
all type defs should remain in rpc-types crate, and we only want to extract all the functions
right @DaniPopes ?
Cargo.toml
Outdated
@@ -45,7 +45,7 @@ members = [ | |||
"crates/transaction-pool", | |||
"crates/trie", | |||
"testing/ef-tests", | |||
|
|||
"crates/rpc/rpc-types-util", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer something other than util
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think compat
or convert
are better alternatives
Don't know why I am getting cyclic dependency errors on cargo test
|
Got it, I would have to separate every function , such that rpc_types does not import compat in any case |
I have made the changes. But +nightly fmt could not be done, due to some issues with my mac . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amazing, this looks great, ty!
I'll take care of the fmt issue
e942e9e
to
081f5f9
Compare
bade788
to
332bd3c
Compare
Ref #3964