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

Voting modularization v3a #397

Closed

Conversation

korczis
Copy link
Contributor

@korczis korczis commented Apr 18, 2015

No description provided.

@botwillacceptanything
Copy link
Owner

☑️ Voting procedure reminder:

To cast a vote, post a comment containing 👍:+1:, or 👎:-1:.
Remember, you must ⭐star this repo for your vote to count.

All comments within this discussion are searched for votes, regardless of the time of posting.
You can cast as many votes as you want, but only the last one will be counted.
(You may consider editing your comment instead of adding a new one.)
Comments containing both 👍up- and 👎down-votes are disregarded.
Comments containing monkeys are disregarded. 🐒 🐵 🙈 🙉 🙊
PR authors automatically count as a 👍 vote.

A decision will be made after this PR has been open for 15 minutes (plus/minus 10 percent, to avoid people timing their votes), and at least 8 votes have been made.
A supermajority of 65% is required for the vote to pass.

NOTE: the PR will be closed if any new commits are added after:
becae55

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.64%) to 27.08% when pulling becae55 on korczis:voting-modularization-v3a into b593b14 on botwillacceptanything:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.64%) to 27.08% when pulling becae55 on korczis:voting-modularization-v3a into b593b14 on botwillacceptanything:master.

@Prior99
Copy link
Contributor

Prior99 commented Apr 18, 2015

👍

@samis
Copy link
Contributor

samis commented Apr 18, 2015

👍 even though I'm not exactly sure what / why exactly this is needed.

@SpenserJ
Copy link
Contributor

I'm not sure everything needs to be defined on the voting object, and I'd rather reuse the minVotes and majority in the config when calculating the guaranteed result

@dbpokorny
Copy link
Contributor

👎 Please wait a minute. I find the voting to be relatively complex. It is already at, say 4th grade level math, and I'd prefer it be around 2nd grade level math: listing the pairs (#yes votes,#no votes) as a key to a "votingPolicy" object whose values indicate the behavior the bot is going to take. {"0-0":"wait","0-1":"wait","1-0":"wait",..."6-0":"win","0-4":"lose","4-4":"wait"}

That way, decideVoteResult just does a table lookup and the entire voting policy is sitting in the config as a table. No addition, no division, no comparison, just lookup into table. In terms of implementation, I've used string keys since javascript (unlike python) has no hashable pair type.

It is ugly, but there is no doubt as to what the bot is going to do. For diagnostics, the http endpoint /votingpolicy could display a simple HTML table that tells the user what behavior the bot takes given a particular number of yes and no votes. It does take more space in the configs (so it could go in, say, config.voting.js or voting.config.js) but it is simple to implement and explain. If colors are used to code bot actions, then a visual inspection of /votingpolicy should be sufficient for the users to agree that there are no eggregious errors in the bot's voting policy.

@samis
Copy link
Contributor

samis commented Apr 18, 2015

@dbpokorny That's a very interesting idea. I would upvote a PR for that.

@shalecraig
Copy link
Contributor

👎

I prefer voting configuration to be in the code, not in the config.

Config is gitignore'd, because it's typically stuff you wouldn't want checked in. This should be checked in.

@SpenserJ
Copy link
Contributor

Agreed, that voting should be in code. A table of vote results also makes it much harder to change the values later on. We would have to rebuild the voting table, if we wanted to change minimum votes to 10.

@shalecraig
Copy link
Contributor

Ugh, just realized that we're doing that with the rest of config.

I have no idea what our story is here.

The below feels like the ideal, but I have no idea what we're doing right now. I know things are happening, but I have no idea where we are from implementation.

  • Setup a config.js (named whatever) that's versioned. It has things like config.js.
  • Create a secrets.js that's gitignored. It has things like OAUTH secrets.
  • Add a line from config.js that merges all attributes exported from secrets onto config.
  • Log the names of fields that were changed.

@dbpokorny
Copy link
Contributor

"A table of vote results also makes it much harder to change the values later on"

I see this as a feature. Changing the voting should be hard, and it should be a big deal.

@korczis
Copy link
Contributor Author

korczis commented Apr 18, 2015

Not understand what table are you talking about.

I just moved constants which were at the beginning of voting.js file to config.

@SpenserJ
Copy link
Contributor

@korczis I think @dbpokorny is proposing that we build a table of predefined vote results. If a vote has 3 yes, and 3 no, it is hardcoded to stay as pending.

@timweightman
Copy link
Contributor

@SpenserJ I think the table could be simplified further, to only include predefined pass or fail results, and all other cases being inferred as 'wait'.
Having said that, I'm not in favor. 👎
Math is a well-established language that can be learned. It is the right way to determine the outcome of a vote.

@OLef1air
Copy link
Contributor

👎

@ghost
Copy link

ghost commented Apr 19, 2015

👍

@timweightman
Copy link
Contributor

In hindsight my downvote was about the propose table. I do not mind the code as submitted in the PR.
Sorry to backflip, but 👍

@dbpokorny
Copy link
Contributor

👍 OK but can the hard wired condition (highestVote < config.voting.guarantedResult) be refactored so that canDecideVote is a function like decideVoteResult? The reason is so that /votingpolicy http endpoint can then display the vote map as it is currently reflected in the bot's voting behavior by calling canDecideVote and decideVoteResult. Colored squares in a matrix could be used instead of letters so that, say yellow means not possible to decide yet if PR wins or loses, green means PR wins, red means PR loses.

@botwillacceptanything
Copy link
Owner

👍 The vote passed! This PR will now be merged into master.


Tallies:
👍: 6 (75%)
👎: 2 (25%)

These following 1 voter(s) were not stargazers, so their votes were not counted:

Be sure to ⭐star the repository if you want to have your say!

@botwillacceptanything
Copy link
Owner

⚠️ Error: This PR could not be merged

The changes in this PR conflict with other changes, so we couldn't automatically merge it. You can fix the conflicts and submit the changes in a new PR to start the voting process again.

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

Successfully merging this pull request may close these issues.

None yet

10 participants