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

fix: edit example error on utils-bn #3770

Closed
wants to merge 2 commits into from
Closed

fix: edit example error on utils-bn #3770

wants to merge 2 commits into from

Conversation

cokia
Copy link
Contributor

@cokia cokia commented Nov 3, 2020

Description

corrected the error in the web3-utils document, in the example of BN.

Run:
new BN('0xea').toString();

Expected behavior
"234"

Actual behavior
"3450"

PS: web3.utils.toBN('0xea') returns "234" correctly

The issue was generated here. #3762

@coveralls
Copy link

Coverage Status

Coverage remained the same at 78.053% when pulling 08e62fb on cokia:1.x into edf481d on ethereum:1.x.

@github-actions
Copy link

github-actions bot commented Jan 3, 2021

This PR has been automatically marked as stale beacause it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Jan 3, 2021
@cokia
Copy link
Contributor Author

cokia commented Jan 6, 2021

this is not stale PR.. :(
please recheck it 👍

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ GregTheGreek
❌ cokia
You have signed the CLA already but the status is still pending? Let us recheck it.

@GregTheGreek
Copy link
Contributor

GregTheGreek commented Jan 6, 2021

@cokia since we migrated the repo to chainsafe, you'll actually need to fill out our CLA

Actually it looks like cla was added by automatically by our org rules. We will be removing that for web3.js

@github-actions github-actions bot removed the Stale Has not received enough activity label Jan 7, 2021
@cokia
Copy link
Contributor Author

cokia commented Jan 8, 2021

In conclusion, do I not have to agree to the CLA up there?

@GregTheGreek
Copy link
Contributor

No please ignore it, ever since the repo transfer things got a little weird

@cokia cokia closed this Jan 11, 2021
@GregTheGreek
Copy link
Contributor

@cokia Is there a reason this was closed?

@cokia
Copy link
Contributor Author

cokia commented Jan 11, 2021

I mistook this for a merged Pull Request.

@cokia cokia reopened this Jan 11, 2021
@GregTheGreek GregTheGreek added 1.x 1.0 related issues Documentation Relates to project wiki or documentation labels Jan 26, 2021
@vicnaum
Copy link

vicnaum commented Jan 26, 2021

Actually, the correct value is 234.

It should had been corrected to:
new BN('ea').toString();

@spacesailor24
Copy link
Contributor

Actually, the correct value is 234.

It should had been corrected to:
new BN('ea').toString();

This returns NaN without the 0x prefix

I'm going to close this because running:

const BN = require('bignumber.js')

console.log(new BN('0xea').toString())

returns 234

@vicnaum
Copy link

vicnaum commented Feb 1, 2021

With bignumber.js - probably yes (I didn't check).
But I thought the default in web3 is BN.js?

With these two big number libraries - there's always some confusion...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Documentation Relates to project wiki or documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants