-
Notifications
You must be signed in to change notification settings - Fork 37
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
Code metadata enhancements #353
Code metadata enhancements #353
Conversation
remove whitespace + change vars to const
src/smartcontracts/codeMetadata.ts
Outdated
@@ -100,13 +119,13 @@ export class CodeMetadata { | |||
} | |||
} | |||
|
|||
enum ByteZero { | |||
export enum ByteZero { |
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.
If exported, perhaps we should rename them to be more explicit? E.g. CodeMetadataByteZero
etc.?
Is it required to export them (this is a low-level detail of CodeMetadata
)? Or perhaps they are only exported for the sake of the tests?
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 with the renaming if they really need to be exported. Maybe some other way of doing was to keep the properties private and have setters and getters, or maybe some methods like isUpgradeable(): boolean
. Just a thought.
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.
Good point, seeing it now I dislike it as well. I will propose a new way by placing Byte info in the CodeMetadata namespace
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.
Updated. Byte info is now only exposed through the CodeMetadata namespace, as in: CodeMetadata.ByteZero.Upgradeable
toJson
fromBuffer