chdsbd / kodiak

🔮 A bot to automatically update and merge GitHub PRs
https://kodiakhq.com
GNU Affero General Public License v3.0
1.03k stars 65 forks source link

Restrict merges to PRs older than a certain threshold #839

Closed JoshuaKGoldberg closed 4 months ago

JoshuaKGoldberg commented 1 year ago

Vaguely similar to #323, but not quite the same. Could we require PRs be older than some minimum age before merging them?

A few use cases where this'd be very useful:

I'm very motivated to get this in for the first bullet on auto-updates, so if you think this is something a first-time contributor could tackle, I'd love to send a PR! 💖

chdsbd commented 1 year ago

I think this is doable. Maybe not the easiest change for a new contributor because we'd need some cron-like functionality.

I think we'd store a PR for reevaluation after some period (N) and then have a cron job that runs every minute or something and checks for PRs that are old enough to merge.

Here's how we mark a PR for evaluation: https://github.com/chdsbd/kodiak/blob/444145f26106e748f497746a25bfc2314b8d510e/bot/kodiak/queue.py#L558-L560

Do you imagine this would be configurable to only affect PRs opened by specific users? Would repo users still be able to manually merge the PR before the time period is up?

Also, do you have ideas about what the configuration would look like?

[merge.after_period]
# only enforce for specified usernames.
usernames = ["dependabot[bot]", "snyk-bot"]
# only merge PRs after the specified age for days
days = 14
JoshuaKGoldberg commented 1 year ago

Do you imagine this would be configurable to only affect PRs opened by specific users?

Ideally yes.

Would repo users still be able to manually merge the PR before the time period is up?

Hmm, great question. I would imagine both forms would be desirable. And we'd probably want to be able to configure some list of users (and/or user roles?) that the restriction wouldn't apply to

Also, do you have ideas about what the configuration would look like?

TBH I haven't used kodiak yet, just gotten excited about it after seeing it in the Next.js repo 😄. But from looking through your configuration files, a few thoughts...

JoshuaKGoldberg commented 4 months ago

Closing out my old issues for repos I no longer have context on. If anybody has a need for what this issue was asking about, I'd encourage them/you to file a new issue. Cheers!