Skip to content
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

Stack outputs constructor error handling #1010

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

frisitano
Copy link
Contributor

@frisitano frisitano commented Jul 18, 2023

This PR introduces error handling in the StackOutputs constructor. If the number of elements is less then 16 then an error is returned.

I have left some TODO inline around error handling related to overflow_addrs and error handling in the processor. Some discussion on these points would be appreciated.

closes: #1007

@frisitano frisitano requested a review from bobbinth July 18, 2023 09:51
core/src/stack/outputs.rs Outdated Show resolved Hide resolved
core/src/errors.rs Outdated Show resolved Hide resolved
@frisitano frisitano force-pushed the frisitano-stack-outputs-constructor branch from 3c990d3 to 888c417 Compare July 19, 2023 09:05
@frisitano frisitano force-pushed the frisitano-stack-outputs-constructor branch from 888c417 to a83d9c5 Compare July 19, 2023 09:07
@frisitano frisitano marked this pull request as ready for review July 19, 2023 09:08
@frisitano frisitano requested a review from bobbinth July 19, 2023 09:23
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you! I left one comment about a potential future improvement. Let's add a TODO for it and merge as is.

core/src/stack/outputs.rs Show resolved Hide resolved
@bobbinth bobbinth merged commit 4772e2f into next Jul 20, 2023
@bobbinth bobbinth deleted the frisitano-stack-outputs-constructor branch July 20, 2023 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants