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

Fixup base fee type #6456

Merged
merged 9 commits into from
Sep 29, 2023
Merged

Fixup base fee type #6456

merged 9 commits into from
Sep 29, 2023

Conversation

3commascapital
Copy link
Contributor

@3commascapital 3commascapital commented Sep 25, 2023

Description

Please include a summary of the changes and be sure to follow our Contribution Guidelines.

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 lint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success.
  • I ran npm run test:coverage 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.
  • I have linked Issue(s) with this PR in "Linked Issues" menu.

@3commascapital
Copy link
Contributor Author

3commascapital commented Sep 25, 2023

@3commascapital
Copy link
Contributor Author

3commascapital commented Sep 25, 2023

was build process recently modified? not sure why no jobs were run. @avkos , @jdevcs , @luu-alex, @Muhammad-Altabba anything look odd to you?

@luu-alex
Copy link
Contributor

@3commascapital , test suites need to be approved first, its running now

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #6456 (e1c7684) into 4.x (3060994) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##              4.x    #6456   +/-   ##
=======================================
  Coverage   89.39%   89.39%           
=======================================
  Files         212      212           
  Lines        8137     8137           
  Branches     2213     2213           
=======================================
  Hits         7274     7274           
  Misses        863      863           
Flag Coverage Δ
UnitTests 89.39% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
web3 ∅ <ø> (∅)
web3-core ∅ <ø> (∅)
web3-errors ∅ <ø> (∅)
web3-eth ∅ <ø> (∅)
web3-eth-abi ∅ <ø> (∅)
web3-eth-accounts ∅ <ø> (∅)
web3-eth-contract ∅ <ø> (∅)
web3-eth-ens ∅ <ø> (∅)
web3-eth-iban ∅ <ø> (∅)
web3-eth-personal ∅ <ø> (∅)
web3-net ∅ <ø> (∅)
web3-providers-http ∅ <ø> (∅)
web3-providers-ipc ∅ <ø> (∅)
web3-providers-ws ∅ <ø> (∅)
web3-rpc-methods ∅ <ø> (∅)
web3-utils ∅ <ø> (∅)
web3-validator ∅ <ø> (∅)

@3commascapital
Copy link
Contributor Author

seems like it's just node connection issues? not sure how to debug this

@Muhammad-Altabba
Copy link
Contributor

Hi @3commascapital
You just need to press on the Details of the failed job, like this:
Screenshot from 2023-09-27 08-09-31


And, then you can see the command that you can also run on your machine:

failing-job


And if you scroll you can see the logs:
image

@Muhammad-Altabba
Copy link
Contributor

Hello @jdevcs,
Do you think we can update the types? Or do we need to update the documentation?
Thanks,

@jdevcs
Copy link
Contributor

jdevcs commented Sep 27, 2023

Hello @jdevcs, Do you think we can update the types? Or do we need to update the documentation? Thanks,

Its fix in recently merged PR.

@3commascapital
Copy link
Contributor Author

unfrotunately when i run these locally, the task stalls out. this particular script has been running for 10 mins and has not moved forward:

➜  web3.js git:(fixup-base-fee-type) yarn test:e2e:ganache:ws:firefox
yarn run v1.22.15
$ ./scripts/test-runner.sh ganache ws firefox
Node software used for tests:  ganache
Node running on:  ws://127.0.0.1:8545
$ WEB3_SYSTEM_TEST_BACKEND=ganache && ./scripts/ganache.sh start 1
Starting ganache ...
2b20df245e064a2fef0ab9a698dcd7fd3430dc0cd1a5e108ea1852b6f04867c2
docker: Error response from daemon: driver failed programming external connectivity on endpoint inspiring_sanderson (bb0b515e76a372b03dcf811d0eb3459687790375688db7527d793f894e104875): Bind for 0.0.0.0:8545 failed: port is already allocated.
Waiting for ganache...
Waiting for localhost:8545.
Connected!
Ganache started...
$ node ./scripts/gen_accounts.js
$ ~/web3/web3.js/node_modules/.bin/lerna run test:e2e:firefox --stream
lerna notice cli v6.6.2
lerna info versioning independent

 >  Lerna (powered by Nx)   Running target test:e2e:firefox for 3 projects:

    - web3-eth-contract
    - web3-eth
    - web3-providers-http

 ———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

> web3-providers-http:"test:e2e:firefox"

web3-providers-http: $ npx cypress run --headless --browser firefox
web3-providers-http: Need to install the following packages:
web3-providers-http:   cypress@13.2.0

@luu-alex
Copy link
Contributor

@3commascapital looks like you fixed ur textcases, we have some inconsistent test cases that are failing not related to you.

@3commascapital
Copy link
Contributor Author

just wanted to bump this if there is anyone else that needs to approve to move it forward

@luu-alex
Copy link
Contributor

@3commascapital Hey there, thanks for the patience. I think the last thing you need to do is update the changelog with your change and it should be good to go

@3commascapital
Copy link
Contributor Author

thanks @luu-alex . done!

@luu-alex
Copy link
Contributor

2 more changes needed, thanks alot for ur contributions

3commascapital and others added 2 commits September 29, 2023 16:03
Co-authored-by: Alex  <luu.alex98@gmail.com>
@luu-alex luu-alex merged commit 4b445ae into web3:4.x Sep 29, 2023
58 of 63 checks passed
@3commascapital 3commascapital deleted the fixup-base-fee-type branch September 29, 2023 21:34
@3commascapital
Copy link
Contributor Author

❤️ thank you!

@jdevcs jdevcs mentioned this pull request Oct 18, 2023
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