-
Notifications
You must be signed in to change notification settings - Fork 5k
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
feat(docs): Expand web3 config guide #7131
feat(docs): Expand web3 config guide #7131
Conversation
…nsactionPollingInterval
006213d
to
098a245
Compare
Co-authored-by: Junaid <86780488+jdevcs@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great contributions! Just a few more suggestions and we should be good to go. Thank you so much 🙏🏻
```ts | ||
import { Web3 } from 'web3'; | ||
|
||
const web3 = new Web3('http://127.0.0.1:7545'); | ||
|
||
web3.defaultAccount = undefined; | ||
|
||
console.log(web3.getContextObject().config); | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the benefit of these code blocks - they seem repetitive and don't communicate any new information to the reader. I would prefer to get rid of most of them (I will make note of the instances where I think it would make sense to keep them).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you. The original requirement of the issue was to add an example for each config property. Initially, I used a value different from the default to demonstrate other options. However, during the first round of review, it was requested that we use the default value instead. Now, it seems redundant to me as well.
Co-authored-by: Dan Forbes <dan@danforbes.dev>
…rnFormat and defaultCommon
@danforbes Hi, I resolved the review comments. All examples are removed except for the |
Co-authored-by: Dan Forbes <dan@danforbes.dev>
@danforbes Thanks for those suggestions so much 😄. I'm not a native speaker and I appreciate it. I only left one question to discuss |
Description
Fix #6805
This PR expands the web3 config guide, including a detailed explanation for each config property and the code snippet.
transactionConfirmationPollingInterval
is marked as deprecated since I found it useless after this commit and cannot be found in the whole repo right now. Please correct me if I miss anything.contractDataInputFill
in the API doc is changed todata
frominput
.defaultMaxPriorityFeePerGas
.Type of change
Checklist:
npm run lint
with success and extended the tests and types if necessary.npm run test:unit
with success.npm run test:coverage
and my test cases cover all the lines and branches of the added code.npm run build
and testeddist/web3.min.js
in a browser.CHANGELOG.md
file in the root folder.