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

Improving comments for the _burn() function in ERC721URIStorage. #3324

Merged
merged 1 commit into from
Apr 7, 2022
Merged

Improving comments for the _burn() function in ERC721URIStorage. #3324

merged 1 commit into from
Apr 7, 2022

Conversation

mcIovin
Copy link
Contributor

@mcIovin mcIovin commented Apr 6, 2022

Currently the comments for this function are really documenting what the
inherited function does in the parent contract (in other words, they are
describing what happens when 'super' is called). This commit changes the
comments to be more in line with other comments of other inherited
functions, whereby reference is made to the parent contract, but the
comments describe the local version of the function. An example of this
can be seen in ERC721Royalty.sol, where the comments reference the
parent contract, but describe the functionality of what the override is
doing. This seems like a better approach, because if the parent contract
were to every be changed, the comments would not necessarily need
amending in the descendants.

Fixes #3324

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

Currently the comments for this function are really documenting what the
inherited function does in the parent contract (in other words, they are
describing what happens when 'super' is called). This commit changes the
comments to be more in line with other comments of other inherited
functions, whereby reference is made to the parent contract, but the
comments describe the local version of the function. An example of this
can be seen in ERC721Royalty.sol, where the comments reference the
parent contract, but describe the functionality of what the override is
doing. This seems like a better approach, because if the parent contract
were to every be changed, the comments would not necessarily need
amending in the descendants.
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Thank you @mcIovin!

@frangio frangio merged commit 731e199 into OpenZeppelin:master Apr 7, 2022
@mcIovin mcIovin deleted the fix/Inaccurate-comment-in-ERC721URIStorage.sol-#3321 branch April 8, 2022 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants