We're writing new guides, help us improve them!

Hello everyone! We’ve received some feedback regarding our guides and documentation, and are in the process of revamping and updating them as part of our new docsite :sparkles:

I’ve been working on a new-guides branch on the repo, adding links to the relevant pieces of the API reference and updating the content where needed.

Most importantly, the tokens guide has been revamped, including an all-new section on ERC777! :confetti_ball:

edit: We now also have brand-new getting started and access control guides! :tada: Check out the PR!

What do you think about it? What other guides would you like to see? Share your ideas on how to improve them, or even contribute directly by opening a PR!

5 Likes

First of all, the guides are awesome!!

I was talking with a friend this week and we discussed the importance of quality documentation for projects. I would have loved this level of detail when I came into the space (not that I don't love it now, I am still learning every day). Definitely makes it easier to make the most of the OpenZeppelin contracts.

A lot of my feedback below could probably go in a PR (which is likely to be your suggestion :smile:)
There is also some great opportunities to update some of the referenced blog articles to 2019.

Overview - Get Started - Next Steps

As the blog articles are older (late 2017), we may want to preface them with a note, something like:
The following articles provide great background reading, though please note, some of the referenced tools have changed as the tooling in the ecosystem continues to rapidly evolve.
Could we look to take the content from the blogs, update it and add into the guides?

We could do with updating https://blog.zeppelin.solutions/the-hitchhikers-guide-to-smart-contracts-in-ethereum-848f08001f05 as it was last updated in October 2017 for Truffle 3.4.11 and Solidity 0.4.15 which feels like a lifetime ago. Especially references to testrpc.
I'd also be inclined to have deployment to a public testnet via Infura mentioned as an option (I mostly deploy via Infura rather than my own node).

We could also update https://blog.zeppelin.solutions/a-gentle-introduction-to-ethereum-programming-part-1-783cc7796094 which is from November 2017 for the same reasons. Same for https://blog.zeppelin.solutions/designing-the-architecture-for-your-ethereum-application-9cec086f8317 from November 2017 (especially the sections referencing the now sunset Mist)

Guides - Access Control

Examples in OpenZeppelin

For most contracts, We'll use Roles to govern who can do what.

Should this be "For most contracts, we'll use Roles to govern who can do what."

Roles & Role-Based Access Control

For example, a MintableToken could have a minter role that decides who can mint tokens (which could be assigned to a Crowdsale).

Should "MintableToken" be "a mintable Token" (as MintableToken is using the OpenZeppelin MinterRole whilst the example is creating it's own "Minter" Role).

Here's an example of using Roles in our token example above

There isn't a token example above. Should we change to "Here's an example of using Roles in a token"

Currently we only have one line after the MyToken code example, which is a bit lonely on its own.

Should we add to the end of this: "You'll notice that the role associations are always the last arguments in the constructor" "in OpenZeppelin"

Should we mention that OpenZeppelin has implemented a number of Roles and link to the API?
https://new-guides--openzeppelin-docs.netlify.com/v2.3.0/api/access#roles

In the page links on the top right, there is additional space between Examples in OpenZeppelin and Roles & Role-Based Access-Control. I assume it is because of the different heading levels

## Ownership & Ownable.sol
### Examples in OpenZeppelin
## Roles & Role-Based Access Control

Crowdsales

Crowdsale Rate

Should "smallest amount of a currency" be "smallest unit of a currency"

Should we also show 10^18 wei also === 1,000,000,000,000,000,000 wei
18 decimals can be hard for new users to picture, sometimes it can be good to spell it out.

Should we give an example of cents being the smallest unit of dollar based currency (don't want to be confusing or be dollar centric), as well as ETH and wei? Again (in my experience), this concept can be hard for new users.

Would it be simpler for users to use 10^18 than 1e18 in the documentation? Also good to show how this is best represented in JavaScript.

So, if you want to issue someone "one token for every 2 wei" and your decimals are 18, your rate is 0.5e17. Then, when I send you 2 wei, your crowdsale issues me 2 * 0.5e17 TKNbits, which is exactly equal to 10^18 TKNbits and is displayed as 1 TKN.

Is this correct? Isn't the rate 0.5e18 (0.5 * 10^18) rather than 0.5e17?
(assuming my maths is correct :crazy_face:) The updated text would be: "So, if you want to issue someone "one token for every 2 wei" and your decimals are 18, your rate is 0.5e18. Then, when I send you 2 wei, your crowdsale issues me 2 * 0.5e18 TKNbits, which is exactly equal to 10^18 TKNbits and is displayed as 1 TKN."

Where we have "10 ^ 18" for consistency should we show it as "10^18"

Minted Crowdsale

"token that the crowdsale has permission to mint from" should we add (i.e. is asssigned a MinterRole)

Tokens - But First, Coffee a Primer on Tokens

Should Tokens come before Crowdsales in the left hand index (unless it is alphabetical)?

Should we add to: "Sending tokens" actually means "calling a method on a smart contract that someone wrote and deployed" and the smart contract reduces the balance for one address and increases the balance on another address by the same amount?

Standards

Would it help to define ERC as Ethereum Request for Comment and EIP as Ethereum Improvement Proposal?
Also that ERC20 is so named, as it was issue #20 in the EIPs repository (https://github.com/ethereum/EIPs/issues/20). I think people have heard of the term ERC20 but not know how it came to be. I love explaining that as it is no longer some esoteric thing to something rather boring and easy to understand :smiley:

Should we also have a sentence on the power of standards. (not sure of the wording) As the ecosystem support standards, this means that you can create a token based on a standard and wallets can show balances and allow you to transfer it without having to create any special software just for your token. Without standards, software would need to be created for every type of token created.

Constructing an ERC20 Token Contract

Could we migrate the advanced guide from the forum into the guides?

"That's it! Once deployed, we will be able to query the deployer's balance:" Should we add how to do this, or a link to an internal guide or some Truffle documentation on truffle console? (unless there is a simple way that I don't know about)

A note on decimals

Should we use similar words and examples to "smallest unit" in Crowdsales (or link back to it).

ERC721

Typo: "accross"
Typo: "returs"
Typo: "New items can not be created:" should be "New items can now be created:"

Should the references to https://game.com be to an actual existing site or something that definitely doesn't exist (game.com domain exists)

Utilities

Cryptography

Should we include ECDSA in the guide?

Introspection

"check out example usage as part of the ERC721 implementation." can we link to this or show the code snippet?

Misc

For Counter should we mention that it is in Drafts?


Loving the quality of the guides. So much information!!

Some guides I would like to see (if someone wants to volunteer as tribute :smile: otherwise I could have a go) on general development.

There is so many things that the team have created/developed/evolved, that would be great to easily share with the community as OpenZeppelin best practice suggestions. I would have loved to know how to get a development environment, style, get test coverage and deploy the OpenZeppelin way.

Development environment

A suggested development environment. nvm, node, npm, Truffle (or Embark), IDE such as VS Code etc.

Style guide and linting

How to use the same style as OpenZeppelin for Solidity and JavaScript (tests) and how to enforce via Linting

Also naming conventions for files e.g. test files.

Test coverage

How to report on test coverage as OpenZeppelin does. Even a line or two on CI setup.

Deploying to testnets and mainnet

Including using Infura, simple things like using .env for mnemonic and Infura API keys. (otherwise pointing to documentation for Truffle)

Contributing

How to get stuff added to OpenZeppelin e.g. a new contract or utility, how to update guides, or even just fix a typo.

Hi @frederickalcantara @RockmanR @drakon it would be awesome if you have more feedback, especially on the revamped tokens guide.

@CallMeGwei and @semuelle would appreciate your unique perspectives (as student and teacher respectively) on the guides.

I see the new-guides branch was removed from GitHub. So we’re critiquing the master branch now, correct? Which has the new guides…? Just clarifying. I think it might be easier to critique over there than here on the forum ( or tell me otherwise :wink: ).

1 Like

@CallMeGwei Read them here:

https://new-guides–openzeppelin-docs.netlify.com/v2.3.0/tokens

You can give feedback in the forum or do a PR:

https://github.com/OpenZeppelin/openzeppelin-solidity/tree/new-guides/docs

I just put my feedback in the forum but may end up doing a PR.

1 Like

Note sure what you mean, I can see the branch just fine here: https://github.com/OpenZeppelin/openzeppelin-solidity/tree/new-guides

That'd depend on what comments you have. For reporting typos, small wording changes, etc., then it might be a better idea to simply open a PR with the proposed changed. More in-depth discussion, however, e.g. regarding things you think are missing from the guides, or new guides altogether, are IMO better suited here.

Ha. I’m not sure what the deal was, but I didn’t see this branch when I pulled it up the other day. I can see it now just fine.

Ok, I’ll keep broader discussions here and specific fixes there (vs raising issues for them).

A little swamped at the moment, but I do want to help with these in some capacity … even if it’s just giving feedback.

Thanks for the initiative. :slight_smile:

2 Likes

Jeez you're absolutely right, I didn't realize how old those were. We will probably revise them soon, but in the meantime I've added the disclaimer you suggested :slight_smile:

This sounds like a great idea for a basic guide! What standards are, why they matter, why it's usually better to use standards as opposed to rolling out your own spec, etc. Thanks!

:joy: I was offline when I wrote that, so I didn't check. We should definitely target a non-existent site.

Thanks a lot @abcoathup for your contributions! I've also reviewed the wording issues you mentioned :slight_smile: In the future, it might be better to simply open PRs for small wording improvements, so that the editing process is more fluid.

1 Like

In other news, I’ve updated the getting started and access control guides, as well as added the erc20 supply one to a new advanced topics section, and migrated some more forum posts over to a FAQ.

Look forward to hearing your thoughts!

3 Likes

We’ve opened a PR for line-by-line review (like @abcoathup did here), all comments are welcome!

1 Like

I can do one of these. Test coverage sounds good. Where can I publish then?

1 Like

In #general:guides-and-tutorials!

2 Likes

@obernardovieira a Test coverage guide would be awesome.

I know that @itinance said they had an issue with solidity-coverage on their machine.

I’ve had problems too when I’ve previously tried to setup.

2 Likes

Cool, will defenitly do it :slight_smile:
I had enough problems with solidity-coverage. I remember I had to fix it to use with solidity 0.5 and then area started the current PR to support solidity 0.5 oficially, because there was a few PR’s from different people. Funny stories.

3 Likes