-
Notifications
You must be signed in to change notification settings - Fork 224
Use foundry UI in sncast #3396
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
base: 3022-2-message-components
Are you sure you want to change the base?
Use foundry UI in sncast #3396
Conversation
…y-rs/starknet-foundry into 3022-3-remove-print-helpers
…y-rs/starknet-foundry into 3022-3-remove-print-helpers
…y-rs/starknet-foundry into 3022-3-remove-print-helpers
…y-rs/starknet-foundry into 3022-3-remove-print-helpers
…y-rs/starknet-foundry into 3022-3-remove-print-helpers
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.
Integration looks simple enough, except for the number of arguments.
Sidenote, maybe the work for sncast and forge could be separate PRs. It's bit hard to follow in which parts where the most significant changes
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.
General comment for all functions, it could make sense to refactor signatures of some of them so they don't take a million (plus a new one) arguments.
Maybe that should be part of the stack, but we can talk it through at the next meeting.
That's what I'm doing but due to the fact that we're using shared functions across our crates, I had to apply some changes in other crates than sncast :/ |
If that's best what we can do then okay. |
…y-rs/starknet-foundry into 3022-3-use-foundry-ui-in-sncast
…y-rs/starknet-foundry into 3022-3-use-foundry-ui-in-sncast
…y-rs/starknet-foundry into 3022-3-use-foundry-ui-in-sncast
…y-rs/starknet-foundry into 3022-3-use-foundry-ui-in-sncast
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.
Probably is fine, didn't review in detail assuming it's all trivial stuff.
…y-rs/starknet-foundry into 3022-3-use-foundry-ui-in-sncast
…y-rs/starknet-foundry into 3022-3-use-foundry-ui-in-sncast
Towards #3022
Stack:
docs
crate #3409shared
#3405Introduced changes
All printed output from
sncast
is now done through foundry-uiChecklist
CHANGELOG.md