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

add: custom transaction polling interval #4584

Merged
merged 8 commits into from Jan 7, 2022
Merged

add: custom transaction polling interval #4584

merged 8 commits into from Jan 7, 2022

Conversation

sirpy
Copy link

@sirpy sirpy commented Nov 29, 2021 •
edited

Description

allow to set via options the interval which polls for TX status
#2621

Type of change

  • New feature (non-breaking change which adds functionality)

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.

@sirpy
add: custom transaction polling interval
23d1021 
allow to set via options the interval which polls for TX status
nazarhussain
nazarhussain requested changes Dec 2, 2021
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.

Hi @sirpy Thanks for contribution. Please add relevant docs under docs/ folder to reflect the changes.

Also update the CHANGELOG.md to specify what new config been added.

@nazarhussain nazarhussain linked an issue Dec 2, 2021 that may be closed by this pull request
Transaction confirmation poll rate as a constructor option #2621
Closed
@nazarhussain nazarhussain added the Review Needed Maintainer(s) need to review label Dec 2, 2021
@nazarhussain
Copy link
Contributor

@sirpy Did you get a chance to look into feedback I posted earlier. I will encourage you to complete it so we can include this PR in upcoming sprint.

@nazarhussain nazarhussain added Changes Requested Requires further changes 1.x 1.0 related issues labels Dec 9, 2021
@coveralls
Copy link

coveralls commented Dec 9, 2021 •
edited

Pull Request Test Coverage Report for Build 1569176448

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 291 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+2.3%) to 74.938%
Files with Coverage Reduction New Missed Lines %
packages/web3-core-requestmanager/src/jsonrpc.js 1 70.0%
packages/web3-core-helpers/src/formatters.js 6 84.87%
packages/web3-core-method/lib/index.js 10 93.85%
packages/web3-eth-contract/lib/index.js 17 91.47%
packages/web3-core-helpers/src/errors.js 29 1.56%
packages/web3-utils/src/soliditySha3.js 34 3.43%
packages/web3-utils/src/index.js 43 31.38%
packages/web3-utils/src/utils.js 44 9.66%
packages/web3-eth-accounts/src/index.js 107 23.67%
Totals Coverage Status
Change from base Build 1545963131: 2.3%
Covered Lines: 3238
Relevant Lines: 4088
💛 - Coveralls

@sirpy
Copy link
Author

sirpy commented Dec 12, 2021

@nazarhussain updated

jdevcs
jdevcs requested changes Dec 17, 2021
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.

Build / e2e_windows (pull_request) is failing on this PR, needs to be fixed

also could you add test for passing custom transactionPollingInterval

@sirpy
Copy link
Author

sirpy commented Dec 19, 2021

@jdevcs I dont see how this PR is related to this error. In anycase I wouldnt have any idea how to fix it.

@spacesailor24 spacesailor24 changed the base branch from 1.x to wyatt/1.x/4584-windows-test-debug January 7, 2022 02:28
@spacesailor24 spacesailor24 merged commit 61d5070 into web3:wyatt/1.x/4584-windows-test-debug Jan 7, 2022
@spacesailor24
Copy link
Contributor

@sirpy FYI I merged this PR into a branch I just created and opened PR #4672, the Windows test is no longer failing. I don't have an answer as to why this works, but we used this to solve the same issue previously

@sirpy
Copy link
Author

sirpy commented Jan 7, 2022

@spacesailor24 thanks

spacesailor24 added a commit that referenced this pull request Jan 14, 2022
@spacesailor24 @sirpy
@nazarhussain @jdevcs
add: custom transaction polling interval (#4584) (#4672)
b32555c 
* add: custom transaction polling interval (#4584)

* add: custom transaction polling interval

allow to set via options the interval which polls for TX status

* add: transactionPollingInterval to contract

* add: pass transactionPollingInterval to method

* Update CHANGELOG.md

* add: transactionPollingInterval docs

Co-authored-by: Nazar Hussain <nazarhussain@gmail.com>

* Add transactionPollingInterval to web3-eth

* Init transactionPollingInterval tests

* Update docs/web3-eth.rst

Co-authored-by: jdevcs <86780488+jdevcs@users.noreply.github.com>

* Update docs/web3-eth-contract.rst

Co-authored-by: jdevcs <86780488+jdevcs@users.noreply.github.com>

* Move 4672 change to 1.7.1

Co-authored-by: sirpy <hadarr@gmail.com>
Co-authored-by: Nazar Hussain <nazarhussain@gmail.com>
Co-authored-by: jdevcs <86780488+jdevcs@users.noreply.github.com>
@sync-by-unito sync-by-unito bot mentioned this pull request Feb 1, 2022
Closed
@jdevcs jdevcs mentioned this pull request Feb 10, 2022
Merged
This was referenced Feb 11, 2022
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Mar 24, 2022
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdevcs jdevcs jdevcs requested changes

@nazarhussain nazarhussain nazarhussain approved these changes

@spacesailor24 spacesailor24 Awaiting requested review from spacesailor24

@luu-alex luu-alex Awaiting requested review from luu-alex

Assignees
No one assigned
Labels
1.x 1.0 related issues Changes Requested Requires further changes Review Needed Maintainer(s) need to review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Transaction confirmation poll rate as a constructor option
5 participants
@sirpy @nazarhussain @coveralls @spacesailor24 @jdevcs