-
Notifications
You must be signed in to change notification settings - Fork 159
Add docstrings and additional comments #234
Conversation
666c41b
to
b707267
Compare
b707267
to
11abb2e
Compare
contracts/protocol/Getters.sol
Outdated
* | ||
* @param marketId The market to query | ||
* @return The current market info, including: | ||
* - The ERC20 token address |
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.
since we're using AbiEncoderV2 these will be returned as structs with named paramaters. It would be a lot more helpful to see this as:
{
keyA: description of keyA,
keyB: description of keyB
}
* | ||
* @param accounts A list of all accounts that will be used in this operation. Cannot contain | ||
* duplicates. In each action, the relevant account will be referred-to by its | ||
* index in the list. |
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.
it'd be very helpful to see at a glance here the structure of an Account.Info
. Also of the form:
{
keyA: descA,
keyB: descB
}
Same for actions and Actions.ActionArgs
* duplicates. In each action, the relevant account will be referred-to by its | ||
* index in the list. | ||
* @param actions An ordered list of all actions that will be taken in this operation. The | ||
* actions will be processed in order. |
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.
[not this line] it'd be really helpful to have detailed comments on each action type and what all the fields for that struct mean
f86de26
to
babd773
Compare
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.
Excellent 💯
Fixes #199