-
Notifications
You must be signed in to change notification settings - Fork 791
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
Cleanup on external API exposure #925
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
Note: |
packages/vm/lib/index.ts
Outdated
|
||
/** | ||
* The StateManager used by the VM | ||
*/ | ||
stateManager: StateManager |
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.
Shouldn't stateManager
, blockchain
, common
be marked readonly
?
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.
Yes, good point, have added in e50af80. Realized that I actually had a wrong understanding of the Readonly modifier.
…ContractSize readonly properties
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.
Looks great! 👍
Closes #924
Ok, not the big sweeping blow I had hoped for but at least some substantial cleanup on what is exposed to the external API. Renamed some clearly internal members to underscore to better differentiate from the officially exposed functionality, also added
protected
keywords where possible so that these members are then not documented in the API docs.Following reasoning and classifications for the respective member changes:
Categories
I also fixed the docs generation accidentally including the tests along the way and regenerated documentation.