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

Improve ERC20Snapshot documentation #2186

Merged
merged 7 commits into from
Apr 14, 2020
Merged

Improve ERC20Snapshot documentation #2186

merged 7 commits into from
Apr 14, 2020

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Apr 14, 2020 •
edited
Loading

Motivated by recent questions in the Telegram group.

@frangio frangio requested a review from nventuro April 14, 2020 17:06
@frangio
Copy link
Contributor Author

frangio commented Apr 14, 2020

image

These are shown in the documentation, but I don't think we want them to show, right? I think we need OpenZeppelin/solidity-docgen#175 for this.

nventuro
nventuro reviewed Apr 14, 2020
View reviewed changes
Copy link
Contributor

@nventuro nventuro left a comment

Choose a reason for hiding this comment

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

These look great, thanks! I'd consider moving the bit about _snapshot to its own documentation so as to not make the intro overly long, but that's quite subjective.

contracts/token/ERC20/ERC20Snapshot.sol Outdated Show resolved Hide resolved
contracts/token/ERC20/ERC20Snapshot.sol Outdated Show resolved Hide resolved
@nventuro
Copy link
Contributor

Yes, being able to hide overridden functions would be useful.

frangio and others added 3 commits April 14, 2020 15:30
@frangio @nventuro
Update contracts/token/ERC20/ERC20Snapshot.sol
d6ca354 
Co-Authored-By: Nicolás Venturo <nicolas.venturo@gmail.com>
@frangio @nventuro
Update contracts/token/ERC20/ERC20Snapshot.sol
6d654bc 
Co-Authored-By: Nicolás Venturo <nicolas.venturo@gmail.com>
@frangio
move a section of docs to _snapshot docs
6f2415d
@frangio
Copy link
Contributor Author

frangio commented Apr 14, 2020

Good idea. Done.

frangio and others added 2 commits April 14, 2020 18:19
@frangio @nventuro
Update contracts/token/ERC20/ERC20Snapshot.sol
f9c0f2a 
Co-Authored-By: Nicolás Venturo <nicolas.venturo@gmail.com>
@frangio
Update ERC20Snapshot.sol
26beb19
@frangio frangio merged commit 3b10205 into OpenZeppelin:master Apr 14, 2020
@frangio frangio deleted the improve-snapshot-docs branch April 14, 2020 22:06
frangio added a commit that referenced this pull request Apr 14, 2020
@frangio
Improve ERC20Snapshot documentation (#2186)
64a1f5e 
Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
(cherry picked from commit 3b10205)
frangio added a commit that referenced this pull request Apr 14, 2020
@frangio
Improve ERC20Snapshot documentation (#2186)
0026cad 
Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
(cherry picked from commit 3b10205)
@Woodpile37 Woodpile37 mentioned this pull request Mar 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nventuro nventuro nventuro left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@frangio @nventuro