-
Notifications
You must be signed in to change notification settings - Fork 152
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
fix(magic-contracts): Add missing docstrings in magic contracts #3457
fix(magic-contracts): Add missing docstrings in magic contracts #3457
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ginowine LGTM! except for this small comment:
NatSpec tags are all optional. That is, if @notice
explains what the function does, there is no need to add a @dev
. only add it when there are extra details
Can you please give it another small iteration, where if a @notice
suffices just remove @dev
Thanks
@salaheldinsoliman. Ok great, thanks. I'll go through it again. |
0346b35
to
12a1467
Compare
could you rename the PR to follow the guidelines on confluence? |
a9188c4
to
98b38d3
Compare
7d46a6c
to
9307c85
Compare
Description of change
Some of the magic contract reference docs are missing information about using the functions. This PR adds Solidity docstrings to the functions of the following contracts:
Links to any relevant issues
`fixes issue #3422
Type of change
Change checklist
Tick the boxes that are relevant to your changes, and delete any items that are not.
develop
branch as the target branch