We should practice selflessness in this experiment. #173
Conversation
github_api/prs.py
Outdated
| path = 'http://github.com/{urn}/pull/{pr_num}.diff' | ||
| diff = api('get', path) | ||
| regex = '\+.+{username}'.format(username=owner_name) | ||
| return re.match(regex, diff) |
There was a problem hiding this comment.
This should be case insensitive
|
Why not let democracy judge that ? |
|
That's what this PR is for, democracy is judging it as we speak. |
|
@rhengles due to @PlasmaPower sneakily grabbing the power earlier :) See #162 for a beautiful story written by @rudehn |
|
I thought about making this very same PR but decided against it, it doesn't stop anyone from just obfuscating and putting their name in. Heck, "deho" + "dson" gets around it. |
|
Good point, but at least it makes the attack more difficult. |
|
I don't like this. It creates a false sense of security. The author can just register another username or just do something like Edit: missed the 2 above comments |
|
A possible solution is to have chaosbot edit all comments and PRs to "blank" them, leaving only the code, hence forcing everyone to look at it before voting. |
|
or make it harder to change the voting rules |
|
Or, perhaps someone other than the author needs to leave a review approving the changes? Obviously doesn't stop collusion but an army of bots mass-upvoting would also circumvent the current process. |
|
@bperson It wasn't an approval, though, and a lot of people probably voted before reading that. If there was a change made requiring someone other than the author approve the changes it would at least show that someone took the time to look at what was changed. |
|
@dehodson so we should not count votes before the review? it seems super flaky to me, we might as well start voting via reviews instead of "reactions" |
|
How about we require one non-author review submitted after the latest push? Edit: and make sure the account is at least one month old. |
|
@bperson, no, no, that's not what I'm saying. I'm saying we need one approval before merging, as well as a positive vote ratio. |
|
Yes, I understood that and you're not solving "a lot of people probably voted before reading that" by doing it. It's like patching a leak with duct tape full of holes :D |
| @@ -23,6 +23,16 @@ def poll_pull_requests(): | |||
| pr_num = pr["number"] | |||
|
Maybe instead of requiring one approval, we should require more approval reviews than rejection reviews, similar to how the base voting works? The difference would be only reviews submitted after the latest push would count. |
:) |
People might, but this project relies on people doing the right thing anyway. If we make it clear that a review should only be submitted after fully reviewing the changes, it might work. |
|
This pull request will prevent people from accidentally submitting their usernames. Which is nice. |
| @@ -1,4 +1,6 @@ | |||
| import arrow | |||
| import re | |||
| import requests | |||
There was a problem hiding this comment.
This import doesn't appear to be used anywhere.
| """ | ||
| path = 'http://github.com/{urn}/pull/{pr_num}.diff' | ||
| diff = api('get', path) | ||
| regex = '\+.+{username}'.format(username=owner_name) |
There was a problem hiding this comment.
This requires there be at least one character between the literal '+' and the username. You probably want .* instead of .+. It'd also be slightly more correct if you used ^ with MULTILINE.
| __log.info("PR %d rejected for selfishness, closing", pr_num) | ||
| gh.comments.leave_reject_comment_too_selfish(api, settings.URN, pr_num) | ||
| gh.prs.label_pr(api, settings.URN, pr_num, ["rejected"]) | ||
| gh.prs.close_pr(api, settings.URN, pr) |
There was a problem hiding this comment.
You could factor out a reject_with_comment function instead of duplicating this logic.
|
This would also prevent legit PRs like #166. |
|
I think it would be useful to require that people be honored by others as a part of legitimate PRs like #166 |
|
🙅 This PR did not meet the required vote threshold and will not be merged. Closing. Open a new PR to restart voting. |
|
Try this again with #153 accepted |
|
I guess I got to use my power after all :D |
Changelog:
owner_name_added_in_diff()which will detect the PR owner's name in the PR diff as an addition.