-
Notifications
You must be signed in to change notification settings - Fork 9
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
docs: suggestions, polish #59
base: master
Are you sure you want to change the base?
Conversation
@@ -1,12 +1,12 @@ | |||
# External interface | |||
|
|||
// Note: is there any point in this file if it's just duplicating the entry points? |
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.
Didn't find this file useful, since the entry points seem to already cover this.
Perhaps we should either dedupe, or discuss which events are emitted instead?
@@ -1,5 +1,6 @@ | |||
# Testing guide |
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.
I saw the issue in #35, but I'm not sure there's much point in having this in the docs since there's not a lot of setup needed. Perhaps we should move this to be with the contract's README?
@@ -276,15 +265,16 @@ Transfer `_amount` tokens from `_from`'s lock by `msg.sender` to `_to` | |||
- Checks that contract has been initialized | |||
- Check that owner’s lock is enough | |||
- Checks that amount input is not zero | |||
// Note: for these next three functions that involve amounts, shouldn't we also have pre-conditions for checking the amount is inside a bound? |
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.
Just wanted to sanity check, since we explicitly state it elsewhere, should we declare checks on the amounts / balances as preconditions here?
Does a round of polish on the current readmes, docs.