Autoapprove/automerge bot PRs?

[kmpaul]

It seems that enabling automerging of PRs on a GitHub repository only allows people to enable automerging on specific PRs. That is, all PRs will not automerge. This creates some noise for us every time dependabot or pre-commit-ci[bot] create PRs to update dependencies or pre-commit configuration. Or am I missing something?

I'm wondering if we can deploy some mechanism for autoapproving and automerging PRs created by bots (e.g., dependabot and pre-commit.ci) when they pass all tests. Something like this:

https://github.com/MobileTeleSystems/mlflow-rest-client/actions/runs/1880831594/workflow

could be deployed on our repos to do this. This workflow demonstrates the alexwilson/enable-github-automerge-action@1.0.0 action and the hmarr/auto-approve-action@v2 action.

[jukent]

@brian-rose I've noticed that you are mostly the one who merges these PRs. How high of a priority do you think it would be to automate this?

[brian-rose]

I do a lot of this merging because I like to clean out my notifications once a day!

I think an alternative to auto-merging these type of basic maintenance PRs would be to allow exceptions to our typical "two review approvals" requirement. Then a single maintainer (like myself) could just quickly hit merge and be done with it.

[brian-rose]

To answer your question @jukent I think finding a better solution to our ongoing maintenance burden is a high priority.

[jukent]

Yeah sounds pretty clear haha

[brian-rose]

We discussed this at today's IWG meeting.

A barrier to auto-merging bot PRs is that recently pre-commit-ci[bot] has been repeatedly trying to update our repos to a pre-release alpha version of prettier, e.g. https://github.com/ProjectPythia/pythia-foundations/pull/317

This creates situations where tests pass but the bot is trying to merge something we don't want.

We have not had problems like that with dependabot so we could turn on auto-merging for those PRs.

We also discussed whether we should get rid of pre-commit-ci entirely, in favor of locally installed pre-commit tools for each developer. The consensus was that the local route works well for projects with a core developer team and helps build good habits among the devs. But it adds significant complexity for first-time contributors to a community repo. We decided to stick with pre-commit-ci despite its annoying bot.

It would be great to set a more fine-grained PR review policy, where PRs originating from maintenance bots only need one single review before merging -- while keeping our "two-review" policy for other (usually more content-oriented) PRs. Does anyone on @ProjectPythia/infrastructure know how to do this?

[dopplershift]

There's no way within the GitHub UI to dynamically adapt the number of required reviews. We only get to pick one of them (1 or 2) to be enforced by GitHub.

I'll also note that GitHub's "auto-merge" feature always requires someone to enable it on a given PR. There's no way to set criteria for PRs that should be merged with no maintainer intervention. Once CI passes and reviews are in, in fact, there's no way to even enable automerge, it's just a way for a maintainer to make sure something gets automatically merged once the other criteria are in.

[brian-rose]

At today's IWG we decided on a new approach: select 2 reviewers for every PR (round-robin requests from the GitHub teams), but only require 1 review before merging.

We will continue our best practice of expecting at least 2 reviews on most non-trivial PRs, but the onus will be on us to identify these as such. The trivial bot-initiated PRs can get merged after 1 review.

[brian-rose]

I think we decided to close this issue now that we've implemented the change in the branch-protection rule (1 review rather than 2 required).

[brian-rose]

Quick update, it turns out that the change to branch-protection rule only happened in the Foundations repo. This morning I changed it for this repo as well and made sure the two are consistent.