-
Notifications
You must be signed in to change notification settings - Fork 131
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
add method to encode tuple element #624
base: master
Are you sure you want to change the base?
Conversation
439daff
to
dcbcc5a
Compare
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.
overall looks good and correct in implementation, need changelog entry and resolve discussion thread on encode
method.
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.
Generally makes sense, but I'd like to see sufficient test coverage too. (Check out test_indexTuple
in pyteal/ast/abi/tuple_test.py
for coverage of _index_tuple
for inspiration)
I am still opinionated on this one tho, after some more thoughts. Maybe I am wrong. This |
barnjamin can you please speak with Hang to elaborate his concerns regarding multiple instance requests of ReturnedType |
@ahangsu correctly notes that if someone were to call something like However, this behavior is already present in the current release. While it isn't ideal I think it shouldn't be a show stopped for this pr. Perhaps another pr that improves how a ReturnedValue caches its computation (via scratch var or otherwise?) is warranted. |
Co-authored-by: Hang Su <[email protected]>
2852bef
to
abe8203
Compare
I forgot about this one, assuming the conflict is resolved is it good to merge? |
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
I just want bytes, no need for other scratch vars or lambdas
diff from a program where i switched to this: