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.

Sign up for GitHub

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

Jump to bottom

4735/4.x alpha misc items #4879

Merged
merged 97 commits into from
Apr 26, 2022
Merged

4735/4.x alpha misc items #4879

merged 97 commits into from
Apr 26, 2022

Conversation

nikoulai
Copy link
Contributor

Description

#4735

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.

spacesailor24 and others added 24 commits March 16, 2022 21:18
@spacesailor24
WIP Web3Eth.sendTransaction tests
8e0f3e0
@spacesailor24
Merge branch '4.x' into wyatt/4.x/4810-web3-eth-promievent-tests
98100a4
@spacesailor24
Update use of ReceiptInfo and TransactionReceipt to ReceiptInfoFormatted
b0c4448
@spacesailor24
Add optional data field to BaseTransaction
a8e7858
@spacesailor24
Init sendTransaction test and fixture
32731f8
@spacesailor24
Remove old sendTransactionValidData
b0099ac
@spacesailor24
Remove no longer needed imports
f2b59bb
@spacesailor24
Remove errornous sendTransaction test
55409e9
@spacesailor24
Init getGasPricing tests for sendTransaction
467fc9f
@spacesailor24
Init wait_for_transaction_receipt.ts
d879b70
@spacesailor24
Init watch_transaction_for_confirmations.ts
0bba805
@spacesailor24
Import waitForTransactionReceipt and watchTransactionForConfirmations…
9863c0f 
… from utils/
@spacesailor24
Init waitForTransactionReceipt and watchTransactionForConfirmations t…
9788911 
…ests
@spacesailor24
Fix promiEvent bug with sendSignedTransaction
e66361f
@spacesailor24
Use HexString for expectedTransactionHash instead of string
bd7ff72
@spacesailor24
Remove jest.useRealTimers
ae0d00b
@spacesailor24
Init sendSignedTransaction tests
9314e6f
@spacesailor24
Update use of ReceiptInfo to ReceiptInfoFormatted for sendSignedTrans…
d5bc84e 
…action
@spacesailor24
Merge branch '4.x' into wyatt/4.x/4810-web3-eth-promievent-tests
d603378
@spacesailor24
Remove unused import ReceiptInfo
a09482b
@nikoulai
Delete all .npmrc files
5e8b3d5
@nikoulai
Delete symbolic links too and update gitignore
5f3101b
@nikoulai
Remove Eth Transaction package
37f216f
@nikoulai
Add license string in all ts files
127e755
@nikoulai nikoulai added the 4.x 4.0 related label Mar 22, 2022
@nikoulai nikoulai self-assigned this Mar 22, 2022
@nikoulai
Copy link
Contributor Author

Please, point out any typos you are aware

@nikoulai nikoulai requested review from spacesailor24 and luu-alex and removed request for luu-alex March 22, 2022 15:04
nazarhussain
nazarhussain requested changes Apr 13, 2022
View reviewed changes
.gitignore Outdated
Comment on lines 37 to 39

# npm config file
.npmrc
Copy link
Contributor

Choose a reason for hiding this comment

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

This can apply to every dot file in the project. As every other dot file .npmrc contains project level configuration related to project which every contributor must follow. And for such configuration I don't think ignoring is a good practice.

.lintstagedrc.json Outdated Show resolved Hide resolved
packages/web3-core/.eslintignore Outdated Show resolved Hide resolved
packages/web3-core/package.json Outdated Show resolved Hide resolved
avkos
avkos requested changes Apr 18, 2022
View reviewed changes
packages/web3-eth-ens/package.json Outdated Show resolved Hide resolved
packages/web3-providers-ipc/package.json Outdated Show resolved Hide resolved
packages/web3-providers-ws/package.json Outdated Show resolved Hide resolved
packages/web3-validator/package.json Outdated Show resolved Hide resolved
tools/eslint-config-web3-base/package.json Outdated Show resolved Hide resolved
tools/eslint-config-web3-base/ts.js Outdated Show resolved Hide resolved
@nikoulai
Copy link
Contributor Author

Changed files:

  • Remove and block .npmrc via .gitignore
    .gitignore
    packages/*/.npmrc

  • Fix typos in ReadMe of packages
    packages/web3-eth-iban/package.json

  • Remove Eth Transaction Package as this functionality was decided to move in Web3 Eth
    packages/web3-eth-tx/*

  • License String in all code files
    tools/eslint-config-web3-base/ts.js
    yarn.lock
    package.json

nazarhussain
nazarhussain approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@nazarhussain nazarhussain left a comment

Choose a reason for hiding this comment

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

Apart from open point https://github.com/ChainSafe/web3.js/pull/4879/files#r835745933 looks good to me. On that I would look for opinion from @spacesailor24 and @luu-alex

@luu-alex
Copy link
Contributor

Aside from @nazarhussain's comment, I added my thoughts. but other than that lgtm

@nikoulai nikoulai requested a review from avkos April 20, 2022 18:13
@nikoulai
Copy link
Contributor Author

@jdevcs are you fine with not ignoring .npmrc?

@jdevcs
Copy link
Contributor

jdevcs commented Apr 22, 2022

@jdevcs are you fine with not ignoring .npmrc?

Its good security practice to ignore .npmrc for avoiding accidental commit of npm auth tokens.
But if team want to keep this file, its ok.

jdevcs
jdevcs approved these changes Apr 25, 2022
View reviewed changes
Copy link
Contributor

@jdevcs jdevcs left a comment

Choose a reason for hiding this comment

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

lgtm, except you need to fix merge conflicts.

@nikoulai nikoulai merged commit d276228 into 4.x Apr 26, 2022
@nikoulai nikoulai deleted the 4735/4.x-alpha-misc-items branch April 26, 2022 10:41
@Jehovaholyspirit01
Copy link

Jehovaholyspirit01 commented May 26, 2022 •
edited
Loading

Description

#4735

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.

All commands to be kept except add all to a different account of mine that will have my verified detailed bussiness and individual information uppsdated bitpay 5277 1300 2010 6779
RONNY GIRON 01/27 cvv041 gironronny1@gmail.com and update my libndogo

@Jehovaholyspirit01
Copy link

Jehovajesuschristholyspirit tag on GitHub please

@Jehovaholyspirit01 Jehovaholyspirit01 mentioned this pull request May 26, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luu-alex luu-alex luu-alex approved these changes

@nazarhussain nazarhussain nazarhussain approved these changes

@avkos avkos avkos approved these changes

@jdevcs jdevcs jdevcs approved these changes

@spacesailor24 spacesailor24 Awaiting requested review from spacesailor24

Assignees

@nikoulai nikoulai

Labels
4.x 4.0 related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4.x Alpha Misc Items
7 participants
@nikoulai @luu-alex @jdevcs @Jehovaholyspirit01 @nazarhussain @avkos @spacesailor24