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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix default threshold in motion #9419

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

h4x3rotab
Copy link
Contributor

From the description, the default motion threshold should be "50%+1". However the old implementation was ceil(50% council). When the council has an even member size, it produces 50% council, but not 50%+1. This commit fixes it.

From the description, the default motion threshold should be "50%+1".
However the old implementation was `ceil(50% council)`. When the council
has an even member size, it produces 50% council, but not 50%+1. This
commit fixes it.
@jacogr
Copy link
Member

jacogr commented May 8, 2023

I really do appreciate the PR (especially since the other has been stuck in limbo forever), it is just that the underlying issues remain.

The bigger issue I see with only this part is that it also has effects on e.g. treasury and everywhere council is involved. And then the thresholds has the same issue everywhere else as well, e.g. tech comm was also a problematic (issue logged) case.

The only outstanding issue (merge master in again this morning) of #6924 is that I have not had a gap to configured a dev chain with even thresholds to test. So if there is an ok from somewhere that "it works", it is reasonable to merge.

TL;DR Holdup on the linked PR is not code, rather "it does what it is supposed to in tests".

@h4x3rotab
Copy link
Contributor Author

Thanks for the confirmation. Moving to #6924 and I'm happy to provide help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants