-
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
feat: Introduce NodeBuilder #5824
Conversation
06a5c52
to
47500cc
Compare
now it really is a copy
6b99292
to
0dac9bd
Compare
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.
this is great!
I like this a lot.
I have a few ideas for followups, but these will be related to better node type abstraction.
with the builder we now have the flexibility to make incremental changes.
as immediate followup, I suggest we refactor the bin/reth crate layout:
group commands in commands folder,
move NodeBuilder and related to node
, this is prep for extracting that code to a standalone repo
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 like this a lot.
this unblocks more generalization.
we can now make more incremental changes and improve extensions and UX
This implements the node builder designed in #5693, and adds a simple block number test with the following UX:
TODO:
For follow-up:
Fixes #5693