-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Eprod-974] Process Poll Timer #3
Conversation
} | ||
|
||
/// Closes the poll | ||
pub fn close(self, result: PollResult) -> ClosedPoll { |
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.
Wouldn't it be better to derive the result of the poll here instead of giving it from outside? The current approach opens a way to create a closed poll with more no
votes, that is still accepted. Of course, it will probably not happen, but it's usually better to have type system protect you from creating an invalid instance.
It's not a blocker, so I will approve the PR, but consider refactoring this.
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.
The PollResult is provided from outside because here we don't know the poll-approval policy. As of today, we require yes-votes to be more than no-votes, but we could apply different strategies like "All yes-votes", "At least 60% yes-votes" and so on. If needed, this can be refactored in the future to accept an "ApprovalStrategy" instead of a PollResult
No description provided.