-
Notifications
You must be signed in to change notification settings - Fork 209
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
Take it easy #275
Take it easy #275
Conversation
settings.py
Outdated
@@ -73,4 +73,4 @@ | |||
|
|||
# PRs that have merge conflicts and haven't been touched in this many hours | |||
# will be closed | |||
PR_STALE_HOURS = 24 | |||
PR_STALE_HOURS = 48 |
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.
Maybe there's a middle ground here? I haven't seen this misfire.
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.
36?
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.
Sure.
This is a good idea. For example, I would do this: Before 1 hour:
Before 2 hours:
Before 3 hours:
Before 4 hours:
Before 5 hours:
After 5 hours:
|
@rhengles The threshold should always be at least 2 (probably 3) to approve, as otherwise an insta-merge attack is possible. Yes, we fixed that one bug, but 3 votes removes a lot of attack possibility. |
@rhengles It'd keep the current voting window and thresholds as they're currently defined, they work pretty well I think. Increasing the voting window as project gets older (as @J0hn- suggested and if I understand correctly?) can count on my vote. |
Good point. Then we could do: After 4 hours:
Either way, we must make sure to count the time from the last commit. @hongaar the point is to not to unnecessarily hold a PR which has a lot of traction or rejection. |
Hmm... I don't think holding a PR is a problem per se. It will get merged eventually (after window passes) so it'd be just some good exercise in patience. If we rush the merging of PRs we'd be at risk of another attack at 'low traffic hours' from contributors with some friends. On the other hand, we're already at risk of such an attack anyway... kind of... |
IMO that window is too big |
Pro tip: add a |
We can debate about the « too big » for the voting window! |
I don't think that 12h is too big at all. More mature software projects use windows way bigger than that. It'll just require a bit more coordination, as one merge can easily cause conflicts in upcoming proposals, etc. I'd say a logarithmic curve from 3h --> 24h long term (+- 1y) |
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.
Great work everybody!
🙆♀️ PR passed with a vote of 21 for and 1 against, with a weighted total of 20.0 and a threshold of 6.1. See merge-commit d1cfad2 for more details. |
#309: Dynamic voting window 🚀 Description: This PR bounces on suggestions made in #275 That is to say : - The voting window is now the same as usual (this is now call as "initial_voting_window") - Two reasons can now extend the voting window (call as "extended_voting_window") : « There are two scenarios that the voting window is needed to be long; where it is being hotly debated, and where it gets no votes for many hours. » And what is the value of the "extending_voting_window" ? The value depends of the time since the creation date of the repo, following this rule :  When the repository is less than 3 days, the extending voting windows is 3 hours When the repository is more stable and advanced, 16 days or more, it's 9 hours :ok_woman: PR passed with a vote of 11 for and 0 against, with a weighted total of 11.0 and a threshold of 6.2. Vote record: @Ealhad: 1 @J0hn-: 1 @Leigende: 1 @akilegaspi: 1 @aslafy-z: 1 @eamanu: 1 @jakejdavis: 1 @paris-ci: 1 @rhengles: 1 @rudehn: 1 @tarunbatra: 1
I think the voting window is too short, especially for people in different timezones than the US.
After the initial interest in chaosbot is slowly starting to fade, it makes sense to slow down the time limits in the voting process just a bit.