-
Notifications
You must be signed in to change notification settings - Fork 523
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
shivarthu-milestone-2 #1209
base: master
Are you sure you want to change the base?
shivarthu-milestone-2 #1209
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.
Thanks for the delivery @amiyatulu a couple of initial comments:
- The application states that React will be used for the front-end. Instead, the front-end delivered is in Leptos/Rust. Can you state the reasons for the change? As a change like this typically requires an amendment to be approved ahead of time.
- On the deployed Leptos app, when I select the "test net" button it doesn't do anything. Is this supposed to be working?
- There is a ton of template text still left in the readme from the Substrate Node Template. Can you please remove the text that doesn't apply for this project? Most of it can be removed since this template has been deprecated, leaving a lot of broken links.
- Some unit tests are failing, if you could look into fixing these. Thanks!
I moved to leptos, because of rust compile time checking and rust safety. It will be easier to maintain and debug rust code in long run. "test net" button doesn't do anything, currently substrate is not hosted, so its not testnet now. I updated the code yesterday with some cleanup, so some tests are failing. Will update the tests and readme soon and get you back. |
Hi, updated the unit tests and cleaned up the readme. |
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.
Hi @amiyatulu thanks for the fixes, much appreciated. The unit tests are all passing now. Moving on, there are a ton of cargo clippy
warnings, can you take a look to see if any of these are easily fixable?
Additonally, when running cargo test --features runtime-benchmarks
there are quite a few benchmarking tests failing. Can you please take a look at the evaluation and provide fixes when possible.
Since it's the last milestone, would be great if we could button these up as much as possible. Thanks!
Hi @keeganquigley Thank you for your reply. Working on benchmarking, and will get back to you when done. |
Thanks @amiyatulu sounds good. Also, FYI I wrote the wrong command for benchmarks, the updated one is what I actually used |
Milestone Delivery Checklist
Link to the application pull request: w3f/Grants-Program#525 < please fill this in with the PR number of your application.