Automerge workflow

[jakirkham]

In some cases a PR is approved and ready to merge, but is still running on CI. Depending on the tests required, the queue load, etc., this can take a while. For cases like these, it would be helpful to flag a PR as ready to merge. That way as soon as CI passes a bot can come in and merge the PR. This is particularly useful if the reviewer that approved is off doing other things.

We have been doing something like this in conda-forge for a little while now using a custom label. RAPIDS has also recently adopted this process. Am curious if this is something we would be interested in doing in Dask.

FWIW here's the first GitHub action that I found to do this. Though there are several others like it. My guess is anything like this will require some GH token to setup, but I could be wrong about that

[jrbourbeau]

I've not used it yet, but it looks like auto-merging PRs is available directly through GitHub https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/automatically-merging-a-pull-request

[jakirkham]

Interesting! Did not realize there’s integrated support for this in GitHub. Thanks for sharing that James! 😀

[jacobtomlinson]

GitHub added this around the same time RAPIDS rolled its solution. I think GitHub's is a little more basic, as a maintainer you can mark a PR for auto merge on completed checks on a PR-by-PR basis.

I guess the use case is to replace when you comment with "happy to merge on passing CI".

With other solutions auto merging is handled using labels. This is useful because some PRs are created by automations and can add that label immediately.

I've recently proposed we go with this approach for the Docker images in dask/dask-docker#149. Currently we have an automation which checks Conda Forge for new versions of Dask and if it finds one it opens a PR to update the Docker image config. In this PR I want to add an automerge label to that PR and if checks pass have it merge and tag automatically. Similar to how Conda Forge has automated merging lately.

In that case I specifically do not want to auto merge everything. Just automated version bumping PRs. So labels makes sense.

[jrbourbeau]

Yeah, that makes sense in the dask-docker case. I'm looking forward to the bots take care of things : )

FWIW I'm up for trying out automerge functionality as we run into situations where it's useful. For repos like dask/dask and dask/distributed I suspect the easiest thing to do is try out GitHub's built-in version of automerge. For other projects, in particular for their release and/or deploy processes, an auto-tag / auto-merge approach seems reasonable.

@quasiben @jacobtomlinson @jakirkham any objection to me enabling GitHub's auto-merge over in dask/distributed (since it get's less traffic than dask/dask) to see how things go? I'm definitely also open to other suggestions

[jakirkham]

Yep was about to suggest that approach. +1 from me 🙂

[jrbourbeau]

Just checked the "Allow auto-merge" box over in distributed

[jakirkham]

Thanks James 😄

Let's use PR ( https://github.com/dask/distributed/pull/4581 ) as a test case 🙂

[jakirkham]

I think we've learned a lot from our experimentation. For an overview, would read this comment ( https://github.com/dask/distributed/pull/4582#issuecomment-796982949 )

I think one last piece here is whether we want to start requiring CI to pass before merging (to protect against bad auto-merges)

[mrocklin]

I'll add briefly that there is a small psychological cost in having the big red :x: on a PR that passes tests when no maintainer has hit the approve button. Not a huge deal, but it's worth taking into account when making this decision. When I submit PRs to projects that do this I always feel a little bit less welcome.

[jrbourbeau]

In theory requiring CI to pass seems great, but in practice we'll run into flaky tests from time to time (I'm thinking specifically about distributed) where maintainers will need to re-start CI or manually merge. Though this is also motivation for us to stay on top of flaky tests : )

Minor point: GitHub's UI has a lot of red prior to branch protection rules being satisfied (see the image in https://github.com/dask/distributed/pull/4582#issuecomment-796978800). Functionally this isn't a big deal, but seeing a big red "X" saying "Merging is blocked" doesn't give me a good feeling.

My inclination is to hold off on using GitHub's automerge and try the tag-based approach (to avoid the red "X").

Additionally, I just want to mention that I personally don't feel bottlenecked on merging PRs for projects like dask and distributed (though other's opinions are definitely welcome). However as @jacobtomlinson pointed out in https://github.com/dask/community/issues/134#issuecomment-796642781 enabling auto-merging for some release/deployment process can be a nice improvement.

[jrbourbeau]

Ah, Matt beat me to the red "X" point. +1

[jakirkham]

The real value of automerge is making sure PRs go in once they are approved without requiring maintainers come back and manually click the merge button (especially when CI is long running and/or maintainers are already quite busy). Would suggest if we don't like the GitHub workflow, that we figure out what we expect out of an automerge workflow and find an alternative that meets that need

[jrbourbeau]

Maybe we should try the tag-based approach that's used in conda-forge / dask-docker (xref https://github.com/dask/dask-docker/pull/149)