-
Notifications
You must be signed in to change notification settings - Fork 17
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
NNS wrapper extensions #397
Conversation
They can be convenient for non-standard names (netmap.neofs vs contract.smth). Signed-off-by: Roman Khimov <[email protected]>
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.
nice except some naming
rpc/nns/hashes.go
Outdated
} | ||
|
||
// NewFS creates an instance of [Contract] using hash obtained via [InferHash]. | ||
func NewFS(sg ContractStateGetter, actor Actor) (*Contract, error) { |
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.
confusing name to me, i suggest
func NewFS(sg ContractStateGetter, actor Actor) (*Contract, error) { | |
func NewInferred(sg ContractStateGetter, actor Actor) (*Contract, error) { |
or
func NewFS(sg ContractStateGetter, actor Actor) (*Contract, error) { | |
func NewWithInferredHash(sg ContractStateGetter, actor Actor) (*Contract, error) { |
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.
Yeah, I had some doubts about it as well, but decided to try with a shorter one. @carpawell, what'd be your preference?
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, i would die if i did not know what and why FS is here. NewInferred and NewWithInferredHash are not perfect to me either but better
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.
Fixed.
If just need a ContractReader this simplifies things somewhat. Signed-off-by: Roman Khimov <[email protected]>
a3640ef
to
1bcacce
Compare
Inspired by nspcc-dev/neofs-node#2841.