quarkusio/quarkus-github-bot

Schedule to merge after all checks are ok

Opened this issue ยท 7 comments

famod commented

Just like @dependabot merge, I think it would come in very handy to let the bot merge a PR once all checks are green (and the PR is still mergeable).

First I thought that triage/waiting-for-ci should be added automatically, but now I'm not so sure about that.
Maybe better add something like merge-pending or similar?
In any case, if a label is added, it should be auto-removed after the merge.

famod commented

PS: I don't think that this should/would become the default merge-approach, but PRs that are clear to merge and are created later in the evening shouldn't need to wait for merging until someone just clicks the button the next day.

gsmet commented

I have mixed feelings about this. I totally agree for very simple PRs but I sometimes find some issues in PRs that are marked waiting-for-ci. If they were automatically merged, I would probably missed them.

We could add some sort of timer.

Also, we would need to be extra sure the label/comment has been added by a committer.

famod commented

We could add some sort of timer.

You mean a grace period after the checks passed, before merging? Good idea!

Also, we would need to be extra sure the label/comment has been added by a committer.

Good point! Could be limited even further to only you and maybe a few others.

Alternatively/additionally the bot could ping certain people (like you) in case a PR was marked for automatic merging.

Complicating things even more ๐Ÿ˜‰, there could also be some approval workflow...just saying. But at some point it becomes too complex.

Btw, I don't think a label should be the trigger. IMO the label should only function as an information to others. ๐Ÿค”

PS: The merge request should probably be reset in case something is pushed after the merge was scheduled (via a comment), no?

famod commented

Not required anymore, GH now offers an auto-merge feature. See also: https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/auto.20merge

famod commented

GH auto-merge is based on required checks which doesn't really fit for Quarkus.

Complicating things even more , there could also be some approval workflow...just saying. But at some point it becomes too complex.

I think requiring committers to approve the PR using the Branch protection feature in GitHub (as we already do now) should be enough ๐Ÿ˜‰

PS: The merge request should probably be reset in case something is pushed after the merge was scheduled (via a comment), no?

That makes sense. You can also configure in the Branch protection to dismiss any previous PR approvals when the PR changes

famod commented

The thing with branch protection rules is that, AFAICS, you cannot merge in case there are just flaky test failures.

What I meant with approval workflow is that e.g. Guillaume has to approve a scheduled merge request before the bot will actually do the merge. But that doesn't scale very well.

To cover that "I'm often missing PRs before they are merged" problem the bot could notify certain people about the request (without waiting for them to do anything).

Just spitballing here. ๐Ÿ™‚