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

[docs] Replace Truffle/web3 mentions and sample-project with Waffle/ehters #517

Merged
merged 23 commits into from
Apr 28, 2020

Conversation

viarnes
Copy link
Contributor

@viarnes viarnes commented Apr 15, 2020

  • Mentions and links to Waffle/web3 replaced with Truffle/ethers
  • Code samples changed (config, task, test and script
  • Change npx buidler sample project files and code generation
    • Replace msg: Do you want to install the sample project's dependencies with npm (@nomiclabs/buidler-truffle5 @nomiclabs/buidler-web3 web3@^1.2.0)? with Waffle + Ethers + Chai
    • buidler.config.js
    • sample-test.js
    • sample-script.js

@viarnes viarnes requested review from alcuadrado and fzeoli April 15, 2020 16:13
@tmilar
Copy link
Contributor

tmilar commented Apr 15, 2020

We should also update Typescript Support page to match the getting started config for ethers/waffle as well, right? Currently, it's written for truffle/web3

@viarnes viarnes changed the title [docs] Replace Getting Started Truffle/web3 examples with Waffle/ehters [docs] Replace Truffle/web3 mentions and sample-project with Waffle/ehters Apr 17, 2020
@viarnes
Copy link
Contributor Author

viarnes commented Apr 17, 2020

I worked on two different branches (replace-truffle-web3-with-waffle-ethers and replace-truffle-web3-with-waffle-ethers-full-doc) due to a misunderstood but ended up merging them into the first one.

@viarnes viarnes requested a review from fzeoli April 17, 2020 19:09
docs/.vuepress/config.js Outdated Show resolved Hide resolved
docs/getting-started/README.md Outdated Show resolved Hide resolved
docs/getting-started/README.md Outdated Show resolved Hide resolved
docs/getting-started/README.md Outdated Show resolved Hide resolved
docs/guides/compile-contracts.md Outdated Show resolved Hide resolved
```

::: tip
Did you notice the double compile message? When running a script through `npx buidler run`, the `compile` task will be called before running the script, but you can skip this with the `--no-compile` parameter.
Copy link
Member

Choose a reason for hiding this comment

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

this is weird. If compile is running twice because the script is calling it then we should just remove that line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't remove bre.run('compile') because it's used as example on how to use Buidler as a library making the script standalone. Maybe we could pick another task instead of compile to use in the sample script.

I honestly prefer compile + the ::: tip :::.

Copy link
Member

Choose a reason for hiding this comment

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

I see that, but invoking the compile task to yield two compilations and then tell the user to use the --no-compile arg to undo our compile call by disabling the default behavior of compiling is just really convoluted.

@alcuadrado any thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Why not just leave the bre.run as a comment?

Copy link
Member

Choose a reason for hiding this comment

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

It works as an explanation and doesn't do the double compilation thing.

Copy link
Member

@fzeoli fzeoli Apr 27, 2020

Choose a reason for hiding this comment

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

because the line is needed for the standalone script. Let's do this

  • leave the call to bre.run in and simply add a step to comment it out before running the script through Buidler, explaining that that's not needed when running scripts through buidler
  • remove the tip block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the tip block but left the call to bre.run commented because the script is also mentioned on the Getting Started section. Also added a step to uncomment this line out to explain how to manually compile.

docs/guides/truffle-testing.md Outdated Show resolved Hide resolved
docs/guides/typescript.md Outdated Show resolved Hide resolved
docs/guides/typescript.md Outdated Show resolved Hide resolved
@alcuadrado alcuadrado merged commit 1ee75bf into development Apr 28, 2020
@alcuadrado alcuadrado deleted the replace-truffle-web3-with-waffle-ethers branch April 28, 2020 18:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants