Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Close stale PRs with merge conflicts #218

Merged
merged 1 commit into from
May 25, 2017

Conversation

PlasmaPower
Copy link
Contributor

This should take care of some of the PRs rotting at the bottom of the list.

I'm looking at updated_at so that comments will refresh the stale timer. That way, if people are deciding how to approach the merge conflicts, the PR won't close on them.

@reddraggone9
Copy link
Contributor

looks extra close for another attempt to sieze power

Eh, it's too late in the day for that shit. 👍

@PlasmaPower
Copy link
Contributor Author

@reddraggone9 I've considered various means of obfuscating a power seize, but especially with a min vote of 3, it would require sweeping changes. It's simpler just to sneak it into an existing PR un-obfuscated.

@reddraggone9
Copy link
Contributor

Well, I'll be going to sleep in short order, so you've got about eight hours to push your change and get it approved with my name on it. As is usually the case, we have chosen convenience over security 🤦‍♂️

@PlasmaPower
Copy link
Contributor Author

At this rate I doubt I'll be able to collect the votes in 8 hours even without a malicious change.

@reddraggone9
Copy link
Contributor

But it's already approved... It's just waiting on the window to close.

@PlasmaPower
Copy link
Contributor Author

Oops. Apparently GitHub will load new comments without a refresh, but not new reactions.

@chaosbot
Copy link
Collaborator

🙆‍♀️ PR passed with a vote of 6 for and 0 against, with a weighted total of 6.0 and a threshold of 3.6.

See merge-commit 43050ac for more details.

@chaosbot chaosbot merged commit 43050ac into Chaosthebot:master May 25, 2017
@reddraggone9
Copy link
Contributor

It looks like this won't actually work until we stop setting labels every time we poll:
screenshot from 2017-05-25 15-21-31

@PlasmaPower
Copy link
Contributor Author

@reddraggone9 I think it works, some PRs have been closed. If you look at those PRs something happened, like being mentioned in another PR.

@reddraggone9
Copy link
Contributor

@PlasmaPower Can you point to a "This PR has merge conflicts, and hasn't been touched in {hours} hours. Closing." message that @chaosbot has left?

@PlasmaPower
Copy link
Contributor Author

@reddraggone9 Oops, I was just checking that, and I think you're right. Changing to pushed_at only.

chaosbot added a commit that referenced this pull request May 26, 2017
#265: Fix stale PR closing mechanism

Description:
See discussion in #218. I think applying the conflicts label was updating `updated_at`, so the condition would never be met. I've switched it to only looking at `pushed_at`. 24 hours should still be good enough for that.

:ok_woman: PR passed with a vote of 8 for and 0 against, with a weighted total of 8.0 and a threshold of 6.1.

Vote record:
@Germanaz0: 1
@PlasmaPower: 1
@abrad1212: 1
@amoffat: 1
@e-beach: 1
@ozyx: 1
@reddraggone9: 1
@rhengles: 1
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants