-
Notifications
You must be signed in to change notification settings - Fork 212
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
Voting: Make early execution configurable #886
Conversation
a4ecee6
to
757cfed
Compare
757cfed
to
4dab655
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.
Looking good.
I guess changing the parameter could be confusing for users, specially for ongoing votes, so maybe we should ask for the preference during onboarding and maybe burn this new permission by default.
The way I see it, if an org wants to combine votes with early execution and without, they should have 2 separate instances, the same way we do now for different quorums.
I even wonder if it should be better to set this at init time only and remove the setter and the permission.
165ebec
to
7a39f6a
Compare
e674855
to
995dd6b
Compare
Included this for 0.9, the next release that will introduce smart contract changes. Thanks @facuspagnuolo! |
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.
Similar to the comment on the overrule window, what do you think about storing this as part of each vote?
It seems a bit weird that an existing vote can have its early execution behaviour change from underneath it. It causes a rather awkward (and error-prone) UX where you'd want to wait until any sensitive open votes are closed before executing a change to this parameter, rather than doing it whenever you wanted.
It starts to get a bit costly, but both the overrule window and the early execution variables can take up one slot in the Vote struct since they're being introduced together.
995dd6b
to
078ad9d
Compare
@sohkai thanks for the review! Already addressed your comments, please LMKYT! |
27da447
to
ac8dfe0
Compare
Related to aragon/client#590
This PR aims to turn the fact that votes can be early executed once they have already been decided, into a configurable option.