-
Notifications
You must be signed in to change notification settings - Fork 616
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
chore: implement Default
for other databases
#691
Conversation
48e7326
to
c8cfdf4
Compare
use crate::AccountInfo; | ||
use crate::U256; | ||
use crate::{Account, Bytecode}; | ||
use crate::{B160, B256}; | ||
use auto_impl::auto_impl; | ||
use hashbrown::HashMap as Map; | ||
|
||
pub mod components; |
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.
Why this style and not moving pub used componnets
behind pub mod
?
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 don't think there's a style guide for this, but I like when it's mod x; use x::*;
groups, like: https://github.com/alloy-rs/core/blob/d9d0ded3ca4e2dde992dad1afe6410015158d6b1/crates/syn-solidity/src/lib.rs
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 a little bit of a different order.
// use if needed. or maybe even put reexports.
use std::..
// list all modules in one place
mod components;
mod tests;
// reexport module items here.
pub use components::*
Something like this is easier to my eyes: https://github.com/bluealloy/revm/blob/main/crates/revm/src/db/states.rs
having mod *
grouped together.
Just bikesheding, this change in general is nice
ada3361
to
cc7fd5e
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.
Check that ExtDB: DatabaseRef + Default
if it can be replaced with Database
as it is main trait
Other things lgtm!
cc7fd5e
to
06ae1c8
Compare
* chore: implement `Default` for other databases * chore: impl EmptyDB traits manually
* chore: implement `Default` for other databases * chore: impl EmptyDB traits manually
And some code cleanup and docs