-
Notifications
You must be signed in to change notification settings - Fork 176
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
add types e2e test for evm #1374
Conversation
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.
Finally we can have it in the main repo! And having it type-safe is an additional benefit. I noticed a few code quality issues though. Additionally to them there are a lot of commented code. But feel free to ignore my comments.
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.
thanks Junius! i've noticed just a few more issues in readme. but they are minor, you can ignore them if you wish.
evm-tests/README.md
Outdated
|
||
## update dependence for coding | ||
|
||
npm update @polkadot-api/descriptors | ||
```bash | ||
yarn upgrade @polkadot-api/descriptors |
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.
missed this last time and confused now. as far as i understand it, this command does the same as cargo update
in rust - it updates dependencies versions in the manifest. i suppose we wouldn't like it to be happened every time the repo is cloned. otherwise why this command is listed here particularly for this dependency?
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 just found out if we generate a new metadata in .papi which including the new extrinsic, for instance. But the IDE doesn't show the new one. I need run it to refresh. It is little confused. I will remove it and put it in personal notes.
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.
oh, if it's needed to generate metadata, then it's better to keep it, but mentioned it in here.
Description
The PR add e2e test cases for EVM in subtensor. All are implemented in type script.
Related Issue(s)
Type of Change
Breaking Change
If this PR introduces a breaking change, please provide a detailed description of the impact and the migration path for existing applications.
Checklist
cargo fmt
andcargo clippy
to ensure my code is formatted and linted correctlyScreenshots (if applicable)
Please include any relevant screenshots or GIFs that demonstrate the changes made.
Additional Notes
Please provide any additional information or context that may be helpful for reviewers.