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

Rewrite web3-eth-iban #3955

Closed

Conversation

loredanacirstea
Copy link

@loredanacirstea loredanacirstea commented Mar 16, 2021

Description

Part of #3932

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success.
  • I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
  • I ran npm run build and tested dist/web3.min.js in a browser.
  • I have tested my code on the live network.
  • I have checked the Deploy Preview and it looks correct.
  • I have updated the CHANGELOG.md file in the root folder.

toString () {
return this._iban;
};
}

module.exports = Iban;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left module.exports = Iban; instead of export default Iban to avoid breaking changes. What do you suggest?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module.exports is fine for this

@coveralls
Copy link

coveralls commented Mar 16, 2021

Pull Request Test Coverage Report for Build 660165167

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 8 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-2.6%) to 73.702%

Files with Coverage Reduction New Missed Lines %
packages/web3-shh/lib/index.js 1 95.83%
packages/web3-bzz/lib/index.js 2 93.02%
packages/web3-eth-iban/lib/index.js 5 86.49%
Totals Coverage Status
Change from base Build 655426514: -2.6%
Covered Lines: 3277
Relevant Lines: 4208

💛 - Coveralls

@frankiebee
Copy link
Contributor

This is good work! unfortunately it's failing test can take a look and see if i can help figure out why.

@loredanacirstea
Copy link
Author

loredanacirstea commented Mar 16, 2021

1 check fails:

>>>>>>
HTTP:MAINNET getBlock
>>>>>>
web3-shh package will be deprecated in version 1.3.5 and will no longer be supported.
web3-bzz package will be deprecated in version 1.3.5 and will no longer be supported.
Error: Failed to connect to Infura over websockets after 10 tries
    at getBlockWithRetry (D:\a\web3.js\web3.js\windows_test\basic_usage.js:31:15)

I see other PRs failing with the same issue coming from https://github.com/ChainSafe/web3.js/blob/2c5a8ec7bbb678b8e718a28fd5fec7709bacde25/scripts/js/basic_usage.js#L50

@spacesailor24
Copy link
Contributor

spacesailor24 commented Mar 18, 2021

Closing in favor of #3965, there was an update to how Infura links are provided to tests (#3943) and it seems to be causing the failing test

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.

4 participants