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 )

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

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.”

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

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 ) 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”

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

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?

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

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.

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)

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

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

Should we include ECDSA in the guide?

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

For Counter should we mention that it is in Drafts?

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