Skip to content
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

Review notes #20

Open
dpc opened this issue Jul 19, 2019 · 2 comments
Open

Review notes #20

dpc opened this issue Jul 19, 2019 · 2 comments

Comments

@dpc
Copy link

dpc commented Jul 19, 2019

Hi,

I've reviewed this crate using cargo-crev.

Some notes:

  • maybe you should not implement Debug for Seed and Mnemonic. Debug makes it easy to accidentally log them somewhere and thus leak them.
  • there are multiple issues that cargo clippy will point out - nothing serious, but it is a good idea to just do what Clippy says. :)
@vorot93
Copy link

vorot93 commented Jul 19, 2019

Your whole comment comes across as a giant coat rack. Too little substance about this project, too much about giving publicity to your tool.

Just sayin'.

@dpc
Copy link
Author

dpc commented Jul 19, 2019

@vorot93 I'm not aware of "coat rack" phrase, but if you mean that I link to the review to increase visibility to increase the awareness of it, then it's true. Shamelessly. :)

As this is a bitcoin-related crate, a review by a reasonably reputable person is a valuable thing (IMO). I've spent around 20-30 minutes going through the code and reviewing it and I'm letting the author know that i did and what I think could improve, with a cryptographic proof. Some companies would pay me money for it.

So I think it's a fair game. If somebody minds, feel free to delete/close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants