-
Notifications
You must be signed in to change notification settings - Fork 449
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
Update multisig
example doc test
#1198
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1198 +/- ##
==========================================
- Coverage 79.03% 78.98% -0.05%
==========================================
Files 229 229
Lines 8667 8667
==========================================
- Hits 6850 6846 -4
- Misses 1817 1821 +4
Continue to review full report at Codecov.
|
🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑These are the results when building the
Link to the run | Last update: Thu Mar 24 03:19:26 CET 2022 |
@@ -308,40 +308,54 @@ mod multisig { | |||
/// | |||
/// Since this message must be send by the wallet itself it has to be build as a | |||
/// `Transaction` and dispatched through `submit_transaction` and `invoke_transaction`: | |||
/// ```no_run | |||
/// use ink_env::{DefaultEnvironment as Env, AccountId, call::{CallParams, Selector}, test::CallData}; | |||
/// ```should_panic |
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.
Why is the should_panic
needed here?
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.
We can't actually fire()
a call from the examples, so it'll panic (but it's fine, we just need it to compile successfully)
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.
Ah got it. Can you create a follow-up issue for actually running the doc tests in CI?
We've made some changes with how cross-contract call building works since the Multisig
example was last updated. The existing doc tests haven't kept up.
Part of the reason is that they're not run. This is because the examples are
cdylib
s.See the error below when I try and run
cargo test --doc
:Maybe in the future we should find some way to run the doc tests in our examples, maybe
by having
cargo-contract
remove thecrate-type
line or something.Closes #1197.