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.

Sign up for GitHub

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

Jump to bottom

chore: make impl Default for StateBuilder generic #690

Merged
merged 2 commits into from
Sep 7, 2023
Merged

chore: make impl Default for StateBuilder generic #690

merged 2 commits into from
Sep 7, 2023

Conversation

DaniPopes
Copy link
Collaborator

Follow-up to #687, see #687 (comment)

rakita
rakita requested changes Sep 6, 2023
View reviewed changes
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

This is more verbose than it was. Using EmptyDB removes the need to specify the type for default()

@DaniPopes
Copy link
Collaborator Author

This is only true with StateBuilder, which can be changed by making the type generic default to EmptyDB. But I don't see why Default (or other traits in the future) are only implemented for the default type.

@DaniPopes DaniPopes force-pushed the db-default branch 3 times, most recently from 0a9ef87 to acf0008 Compare September 6, 2023 14:46
rakita
rakita approved these changes Sep 7, 2023
View reviewed changes
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

Pending conflicts, but lgtm!

I added State::builder() inside State so StateBuilder doesn't need to be specified in usually flows. So we can do State::builder().build() pattern.

@DaniPopes
Copy link
Collaborator Author

Re Based

@rakita rakita merged commit 1d6a039 into bluealloy:main Sep 7, 2023
@DaniPopes DaniPopes deleted the db-default branch September 7, 2023 23:29
mikelodder7 pushed a commit to LIT-Protocol/revm that referenced this pull request Sep 12, 2023
@DaniPopes @mikelodder7
chore: make impl Default for StateBuilder generic (bluealloy#690)
743d0e2 
* chore: make `impl Default for StateBuilder` generic

chore: update methods

* Update crates/revm/src/db/states/state_builder.rs
Evalir pushed a commit to Evalir/revm that referenced this pull request Sep 14, 2023
@DaniPopes @Evalir
chore: make impl Default for StateBuilder generic (bluealloy#690)
895b0d7 
* chore: make `impl Default for StateBuilder` generic

chore: update methods

* Update crates/revm/src/db/states/state_builder.rs
@github-actions github-actions bot mentioned this pull request Jan 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rakita rakita rakita approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DaniPopes @rakita