Open ConsenSys-Academy opened 2 years ago
PS We did not see your Ethereum address for an optional on-chain certification. Please let us know if you would like that and to what public Ethereum address you'd like it sent to
PS We did not see your Ethereum address for an optional on-chain certification. Please let us know if you would like that and to what public Ethereum address you'd like it sent to
This is my ETH mainnet address for the on-chain certification: 0xB49390AB57ab1376aCCa68e3fF1BDcD2fF76f7Ac
Hi @andybn — Congratulations, your Final Project passed! We'll be approving it on the course itself in a week or two, but here's your feedback in the meanwhile:
Frontend
- "Network not supported" a great error for the user, would be good to include in that error message which network is supported so the user can switch to that one.
- Nice reactive UI, detecting network change, etc.
- Not sure if this is native to the Truffle React box but really like your rotating box for waiting animation
- Really great follow-up information about the successful transaction, very clean!
- The new expense submission form is excellent, very clear -- might want to include what currency unit the expense is in, the form didn't let me submit 0.0001 (I assumed incorrectly it was ETH)
- Really nice flow between submitted an expense and approving it – this is extremely well done!
- One minor thing, I didn't realize clicking on the contract address would take me to the expense group, I assumed it would take me to the contract on etherscan or another block explorer. This is very minor but wanted to give you the feedback.
- Great frontend, overall, @andybn — the React code is really impressive
Design and Security
- Really cool you used a factory pattern, was that hard to incorporate? The deployment cost did not seem that much when I used it on the public network, which is great.
- Great commenting on the contract, helps with comprehension and navigation of the project
- Interesting idea to limit the number of members to reduce gas costs, is that just for initialization costs or preventing costs in the future?
Testing
- Nice setup in the tests, appreciate the deconstruction of accounts and the separation of logic into their own
describe
- Really interesting to define functions like
checkEventTriggered
andcheckGetExpense
, makes reading the tests more clear.- Seems like good initial coverage, any other tests you were thinking about?
Overall
- Really appreciate the planning you did at the beginning of the process, @andybn. Mapping out the different user stories, MVP, and future plans is a very effective way to start building a project. I hope you found it helpful.
- It looks like you submitted your project without realizing we had removed the requirements to have the install, run and test scripts -- apologies you had missed that.
- Overall, @andybn, this is an excellent project. Between the planning, the smart contract layout, the testing and the React frontend, you absolutely nailed it. Thanks for all your hard work, I hope your learning journey was good and you can continue you build on this.
Thank you very much for your feedback:
My idea is to keep improving the app to use it mainly as a learning tool to explore even other libraries and frameworks.
Having said that I had a lot of fun with the bootcamp!. It has been a really holistic introduction to the space. Before improving the project I will go through all the material I still have pending to review since I have been fully focused on the project once I finished the smart contract / tooling section.
I still have tons of things to learn and deep but thanks to the training I have pretty clear that my future as developer will be linked to the blockchain space in one way or another!. :)
Hi @andybn — Congratulations, your Final Project passed! We'll be approving it on the course itself in a week or two, but here's your feedback in the meanwhile:
Frontend
Design and Security
Testing
describe
checkEventTriggered
andcheckGetExpense
, makes reading the tests more clear.Overall