-
Notifications
You must be signed in to change notification settings - Fork 5
bounty -> hackathon #46
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
base: dev
Are you sure you want to change the base?
Conversation
| constructor(address _allo, string memory _strategyName, bool _reviewEachStatus) | ||
| BaseStrategy(_allo, _strategyName) | ||
| { | ||
| REVIEW_EACH_STATUS = _reviewEachStatus; |
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.
- Would it make sense to remove REVIEW_EACH_STATUS and always _processStatusRow ?
- Do the extensions need to have a constructor ?
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.
- Hmmm i think that if someone doesnt want to do some extra processing for each individual then its much cheaper to have
REVIEW_EACH_STATUS = falseand fill in the statuses, it gives a bit of flexibility. - we used a constructor here because we have
REVIEW_EACH_STATUSimmutable, usually extensions shouldn't need a constructor, they could use init function
0xOneTony
left a comment
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.
Great job! 🔥
The BountyExtension is a good idea, however if you include the allocation and distribution logic it makes it more of a complete strategy example rather an extension. I think it would be better to keep the BountyExtension without the allocate, distribute, and _processRecipient logic, so the extension will handle the definition of bounties and creating them.
Then you could have a BountyBaseStrategy which will use the RecipientExtension and the BountyExtension and it will have the basic allocate, distribute and _processRecipient logic.
From there you could have different flavors of BountyStrategies either by making use of the BountyExtension or by inheriting the BountyBaseStrategy.
| /// =============================== | ||
|
|
||
| function __BountyExtension_init() internal { | ||
| // todo: no code? |
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.
yes! in this case if you dont have to initialize anything you can leave it
| /// =============================== | ||
| /// ========= Initialize ========== | ||
| /// =============================== | ||
| function initialize(uint256 _poolId, bytes memory _data) external override { |
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.
dont forget to also initialize the RecipientExtension
| /// ========= Initialize ========== | ||
| /// =============================== | ||
|
|
||
| function initialize(uint256 _poolId, bytes memory _data) external override { |
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.
dont forget to also initialize the RecipientExtension
https://hackmd.io/@98cAvzvOSGOFP9kPfbp1YQ/rJk5NSrxyx