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

[documentation] Clarify privateKeyToAccount private string is hex #2129

Merged
merged 2 commits into from
Dec 17, 2018
Merged

[documentation] Clarify privateKeyToAccount private string is hex #2129

merged 2 commits into from
Dec 17, 2018

Conversation

tcrowe
Copy link

@tcrowe tcrowe commented Dec 15, 2018

What does it do?

#1267 brought up a good point that web3.eth.accounts.privateKeyToAccount privateKey does not specify that it's a hex string. This PR fixes that in the documentation.

Does it work?

screen shot 2018-12-15 at 4 50 09 pm

@coveralls
Copy link

Coverage Status

Coverage remained the same at 83.071% when pulling 5b5b296 on tcrowe:docs-privatekeytoaccount-hex-1267 into 1d9f6c0 on ethereum:1.0.

@joshstevens19
Copy link
Contributor

joshstevens19 commented Dec 16, 2018

Thanks @tcrowe, looks fine to me, we need 2 approvals before we can merge so @nivida can you merge when you get a second.

Just a note @tcrowe if your doing doc updates can you please do it against ethereumProvider branch as that will be the new 1.0.x and default branch once its deployed.

@nivida
Copy link
Contributor

nivida commented Dec 17, 2018

@joshstevens19 That's wrong they don't have to be against the ethereumProvider because I didn't change the documentation there and I will merge the ethereumProvider branch into the 1.0 branch.

@tcrowe Thanks for submitting this PR! :)

@nivida nivida merged commit 4b20fc7 into web3:1.0 Dec 17, 2018
@joshstevens19
Copy link
Contributor

Surely you want to keep your ethereumProvider inline with the latest code so would of assumed you would want it in the ethereumProvider, sorry if that was incorrect 👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants