Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Schedule to merge after all checks are ok #28

Open
famod opened this issue Mar 5, 2021 · 7 comments
Open

Schedule to merge after all checks are ok #28

famod opened this issue Mar 5, 2021 · 7 comments

Comments

@famod
Copy link
Member

famod commented Mar 5, 2021

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
Copy link
Member Author

famod commented Mar 5, 2021

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
Copy link
Member

gsmet commented Mar 5, 2021

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
Copy link
Member Author

famod commented Mar 5, 2021

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
Copy link
Member Author

famod commented Mar 19, 2021

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

@famod famod closed this as completed Mar 19, 2021
@famod
Copy link
Member Author

famod commented Mar 24, 2021

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

@famod famod reopened this Mar 24, 2021
@gastaldi
Copy link
Contributor

gastaldi commented Apr 10, 2021

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
Copy link
Member Author

famod commented Apr 11, 2021

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. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants