-
Notifications
You must be signed in to change notification settings - Fork 396
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
feat: add txlink builder and send special arg #3948
base: master
Are you sure you want to change the base?
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
} | ||
|
||
// AddArg adds a key-value argument pair. Returns the builder for chaining. | ||
func (b *TxBuilder) AddArg(key, value string) *TxBuilder { |
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'm usually against this pattern, but in this specific case, it may be interesting to consider making the value any
, and then trying to convert basic types to a string. wdyt?
func (b *TxBuilder) AddArg(key, value string) *TxBuilder { | |
func (b *TxBuilder) AddArg(key, value any) *TxBuilder { |
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 it's not recommended but but might be more flexible and easy to use. @leohhhn since you used this packages way more than me, do you think it could be helpful?
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'm more for typed approaches; pretty sure someone will try to jam in a struct or slice or other invalid input; we can have this approach when we also allow a pattern like std.Emit(&mystruct)
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
Co-authored-by: Leon Hudak <[email protected]>
Can you modify the render of r |
Related to #3919, this PR introduces a new streamlined way to build transaction links in gno, with built-in support for send values.
New Builder Pattern:
Added multiple ways to specify send values:
SetSend("1000000ugnot")
Call("MyFunc", ".send", "1000000ugnot")
The new builder pattern uses the same Realm logic as the previous one
Priority Rules:
As a refinement, implemented clear priority rules for send values:
AddSend
call winsTest Coverage
Added comprehensive test suite covering: