-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: implement delete account functionality #403
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.
Yes, I'll ensure I resolve every issue before the end of today |
@OnahProsperity can you please review my changes I've address all raised issues |
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.
@Adebisi1234 can you resolve the failing tests
- try running the tests locally to ensure they pass before pushing
you can install ganache and run a local blockchain with HD_WALLET_MNEMONIC="media nerve fog identify typical physical aspect doll bar fossil frost because"; ganache -m "$HD_WALLET_MNEMONIC" --chain.chainId 1337 -l 21000000
then run the tests with go tests -v ./...
@chibie thanks I've been able to resolve all failing test. Couldn't run the test cases previously because spinning up the docker image keeps failing while connecting to postgres |
@chibie are there still some changes I need to implement? Or why is the pr not being merged The failing test case isn't from my implementation |
no, it isn't from your changes.. your pr will be merged soon @Adebisi1234 |
I've implemented delete account route
Also written test for the route
I couldn't test the code locally because I couldn't get the container running with postgres
I'll appreciate your review of the code and testing.
closes #366