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

Feat/use rich printer for init #1548

Merged
merged 11 commits into from
Aug 30, 2024

Conversation

elizabethengelman
Copy link
Contributor

@elizabethengelman elizabethengelman commented Aug 16, 2024

What

Refactor contract init command to usesoroban-cli/src/print.rs.

Closes #1553

Why

To be more consistent without our output, and to reduce duplication.

Known limitations

n/a

@elizabethengelman elizabethengelman force-pushed the feat/use-rich-printer-for-init branch from 2dd1fbc to 1d22954 Compare August 16, 2024 17:45
@elizabethengelman elizabethengelman marked this pull request as ready for review August 16, 2024 17:55
@janewang janewang requested a review from willemneal August 21, 2024 22:04
Copy link
Member

@willemneal willemneal left a comment

Choose a reason for hiding this comment

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

I have some nits, but in general I think that we should have a the root command handle the print.errorln of the errors. The map_err's with print.errorln here are providing context to the error and should be an error case so that the caller would have that context.

@elizabethengelman
Copy link
Contributor Author

I have some nits, but in general I think that we should have a the root command handle the print.errorln of the errors. The map_err's with print.errorln here are providing context to the error and should be an error case so that the caller would have that context.

@willemneal that's a good point - instead of printing the error messages and then swallowing the errors, we should return the error for those cases. I can work on that change in a separate PR.

@elizabethengelman elizabethengelman force-pushed the feat/use-rich-printer-for-init branch from e1a33c7 to 4da9a8d Compare August 29, 2024 14:09
@willemneal willemneal merged commit 79a3aa1 into stellar:main Aug 30, 2024
26 checks passed
@elizabethengelman elizabethengelman deleted the feat/use-rich-printer-for-init branch August 30, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Use rich CLI printer in contract init
5 participants