-
Notifications
You must be signed in to change notification settings - Fork 4
Updated ReadMe with docs #30
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
Conversation
fix example
0xNeshi
left a comment
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.
Excellent work on the README!
First review iteration, left some small comments
| The library exposes two primary layers: | ||
|
|
||
| - `EventScannerBuilder` / `EventScanner` – the main module the application will interact with. | ||
| - `BlockScanner` – lower-level component that streams block ranges, handles reorg, batching, and provider subscriptions. This is exposed to the user but has many edge cases which will be documented in the future. For now interact with this via the `EventScanner` |
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.
We're not really scanning blocks, but block ranges. I'm thinking that maybe BlockRangeScanner is a better name?
That way this becomes a sort of a primitive that we can use to also potentially implement an actual BlockScanner in the future.
Wdyt?
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 agree - we should change it both here and in the code. Happy to do it here unless you want to take over
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 can do it
Co-authored-by: Nenad <[email protected]>
d-carmo
left a comment
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.
LGTM
No description provided.