-
Notifications
You must be signed in to change notification settings - Fork 54
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
fest: support start/stop blocks for querying operator info(pubkey and socket) #334
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.
This looks fine to me. @shrimalmadhur do you want to start enforcing opts arguments so that we don't have breaking changes like this all the time?
yea this is getting very ugly. these seems like optional params to me - if we don't provide them, it takes default values. We should add extensible options without breaking the method arguments. |
Ok @renlulu synced with @shrimalmadhur , we're trying to enforce opts params going forward, such that we don't break client code everytime we add new params to a function. Would you mind refactoring the function signatures everywhere to use a params struct instead that would for now contain the startBlock and endBlock?
Not sure about the exact name, |
Ok, make sense, let me refactor it. |
opts.StartBlock, | ||
opts.StopBlock, |
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 will panic if nil
is passed as opts. Can we change opts to be Opts
instead of *Opts
?
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.
in every function
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.
Thank you!
Fixes #259 and #312.
What Changed?
Reviewer Checklist