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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved documentation for _preValidatePurchase method in Crowdsale #1101

Conversation

dougiebuckets
Copy link
Contributor

Fixes #1096

馃殌 Description

Improved documentation for _preValidatePurchase in Crowdsale. Hopefully this makes it more clear how one can use super in contracts that inherit from Crowdsale in order to extend their validations.

@nventuro nventuro added kind:improvement documentation Inline comments, guides, and examples. labels Jul 21, 2018
@nventuro
Copy link
Contributor

Thanks!

I noticed however you rolled back two typo fixes, and changed SafeERC20 for the standard one, was that intentional or did you add those changes to this PR by mistake?

@dougiebuckets dougiebuckets force-pushed the improve-documentation-for-_preValidatePurchase-in-Crowdsale-#1096 branch from ee549b7 to 1327bbb Compare July 21, 2018 17:41
@dougiebuckets
Copy link
Contributor Author

Thanks for the heads up! Not sure how I did that? Should be all set now.

@nventuro
Copy link
Contributor

Maybe remains from a branch merge? Should be fine now, thanks for the quick PR!

@@ -110,7 +110,10 @@ contract Crowdsale {
// -----------------------------------------

/**
* @dev Validation of an incoming purchase. Use require statements to revert state when conditions are not met. Use super to concatenate validations.
* @dev Validation of an incoming purchase. Use require statements to revert state when conditions are not met. Use `super` in contracts that inherit from Crowdsale to extend their validations.
* @dev Example from CappedCrowdsale.sol's _preValidatePurchase method:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The second line shouldn't have a @dev tag. It somehow messes up Solidity's NatSpec parser. We changed this across OpenZeppelin in #995.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Didn't realize that. Fixed!

@dougiebuckets dougiebuckets force-pushed the improve-documentation-for-_preValidatePurchase-in-Crowdsale-#1096 branch from e6e58c8 to 3058c37 Compare July 22, 2018 11:20
@nventuro nventuro merged commit 2307467 into OpenZeppelin:master Jul 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Inline comments, guides, and examples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add examples of extending _preValidatePurchase in Crowdsale.sol
3 participants