PR Owners can change their minds #77
Conversation
If a PR owner changes their mind & votes against the pull request, that vote should count. Also I went ahead and switched to a gender-neutral pronoun.
github_api/voting.py
Outdated
| votes[pr_owner] = 1 | ||
| # by virtue of creating the PR, the owner casts their vote as 1 | ||
| # unless they later vote against it, in which case we respect that | ||
| if votes.get('pr_owner') != -1: |
There was a problem hiding this comment.
this is incorrect, should be votes.get(pr_owner)
There was a problem hiding this comment.
Thanks, I was just about to point that out.
This line in its current form actually just allows the GitHub user named "pr_owner" to override the actual PR owner's vote.
Also, I feel it's worth pointing out that as of the moment of this review, there are 23 other people who voted for this PR without noticing this mistake.
There was a problem hiding this comment.
OMG This thing needs to have tests… that's going to be interesting.
|
Can't the PR owner already just veto the PR if they choose by closing it themselves? |
|
@Ajedi32 probably but they may as well still have the option. Maybe they end up with misgivings about the pull request but still want it to be decided by a vote. |
|
Related: #82 PR owners can submit code that was not what was voted for and use it as a method to inject unexpected malicious code. |
|
Related #48 somehow got through. |
|
@JordanReiter please resolve conflicts |
#138: Allow the owner of a PR to downvote it Description: Redo of #77 with merge conflicts fixed. :ok_woman: PR passed with a vote of 17 for and 14 against, with a weighted total of 4.6 and a threshold of 1.0. Vote record: @aaronduino: 1 @DasSkelett: -1 @ECrownofFire: -1 @Goofybud16: 1 @INightmare: 1 @Kalabasa: -1 @LasaleFamine: -1 @MarHoff: -1 @PlasmaPower: 1 @Redmega: 1 @akilegaspi: 1 @amoffat: -1 @arunoda: 1 @bperson: -1 @crispmark: 1 @demiters: -1 @dylanaraps: 1 @eukaryote31: 1 @hfern: 1 @hongaar: -1 @ike709: 1 @jackcutting: -1 @jblz: -1 @loic-sharma: -1 @matthewc-mps-aust: -1 @pangeacake: 1 @pchauncey: 1 @serhalp: 1 @swaggy: -1 @syrusakbary: 1 @tarunbatra: 1
|
🙅 This PR has merge conflicts, and hasn't been touched in 55 hours. Closing. Open a new PR with the merge conflicts fixed to restart voting. |
|
Oh... oh my god... it's conscious and has an attitude. WE'RE DOOMED 🤖 |
If a PR owner changes their mind & votes against the pull request, that vote should count.
Also I went ahead and switched to a gender-neutral pronoun.