-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[move-prover] Print warning about the level of Prover support for Sui #14348
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
crates/sui-move/src/prove.rs
Outdated
@@ -92,6 +93,7 @@ impl Prover { | |||
), | |||
); | |||
|
|||
eprintln!("WARNING: the level of Move Prover support for Sui is currently limited; use at your own risk"); |
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.
Could we maybe give a block of descriptions of what things might or might not work? And with what amount of work they might need to do?
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.
The idea makes sense but I am worried that this will change in time. Actually, I am not 100% confident what what does and does not work at this point and I feel like a more generic message would be sufficient to trigger further investigation on the side of a person that still wants to go for it despite the warning.
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.
Maybe then we could just explain out what "at your own risk" entails? Like just explaining: "Not everything is guaranteed to work. Please file an issue if an update breaks your usage"
Something to that effect. Like I want people to know that it might not work, but we should also be receptive if we break something that was working
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 finessed the warning message a bit but in reality we don't have enough resources to encourage people to file issues so I did not include that part...
…MystenLabs#14348) ## Description People keep trying to use Move Prover with Sui and there is currently no easily accessible place where the current level of support is mentioned. Adding a warning at the time when Prover is ran seems like a reasonable thing to do here. ## Test Plan Verified that the warning is printed. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [x] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes A warning about current level of Move Prover support for Sui is now printed when the Prover is ran.
Description
People keep trying to use Move Prover with Sui and there is currently no easily accessible place where the current level of support is mentioned. Adding a warning at the time when Prover is ran seems like a reasonable thing to do here.
Test Plan
Verified that the warning is printed.
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes
A warning about current level of Move Prover support for Sui is now printed when you run the Prover.