-
Notifications
You must be signed in to change notification settings - Fork 246
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
refactor: make optional BlockId params required in provider functions #516
refactor: make optional BlockId params required in provider functions #516
Conversation
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.
the tag is a required argument,
I think the option does not improve the API, because None is equal to Default::default()
or BlockId::latest()
That is true, it is a required parameter in the RPC specification, but the other examples provided in the original issue have the same condition so it's a case of inconsistency a bit. I checked those methods and for those it's also required in the specification. |
I see, then we should change all |
|
In some files previously |
The only improvement is writing |
alternate API designs would be to bifurcate, which seems ok-but-verbose on our end
or to switch to a rpc builder pattern for return values of those types. which seems bad |
…alloy-rs#516) * refactor: make blockId optional in getCode and getUncle * feat: change all Option<BlockId> types to BlockId
Motivation
This issue #514
Solution
Make the
BlockId
optional and use the default value if it'sNone
.PR Checklist