-
Notifications
You must be signed in to change notification settings - Fork 261
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
Proposal - enable automerging capability #2453
Comments
I like the idea. My only concern is indeed that a queue is built up that we don't have a way to manage -- if there's some PR that we urgently need to "jump the queue" we end up waiting until everything above it is merged. However, given that we're applying this approach already with Dependabot, this would give us a way of letting the system inject other PRs into that same "queue" automatically; which without such a change we don't have a way to do today. And we can presumably mitigate the risk of having a queue that is too big and blocks any other PRs from being merge-able by being careful about marking too many PRs as automerge at the same time? (Or leaving such activity to some time period where we expect some quiet time PR-submission-wise?) So on that basis, I'm more in favor than not -- say |
Yes it's not perfect - I wondered if there's a way of only running off-peak, but that would probably require us forking an existing action & adding that capability (not looked thoroughly yet - wanted to float the idea). I feel we have a fair bit of human 'polling; going on to merge our PRs which is wasting time. I'd hope the queue never gets too big as it's now much easier to manage. Technically we could probably subvert the q for something critical by canceling the PR jobs in azure but it's not really a smooth process. |
Signed-off-by: Nigel Jones <[email protected]>
Signed-off-by: Nigel Jones <[email protected]>
Signed-off-by: Nigel Jones <[email protected]>
Automerge is not working with this configuration. The job fails. This may well be down to the action not updating the PR prior to the merge attempt. Have reported to the author. Having looked at the other marketplace options it's not entirely clear any will completely address our needs without building a more complex pipeline. This might be better done in a test repo. Will leave this open to see if there's a simple approach we can take |
Signed-off-by: Nigel Jones <[email protected]>
#2453 update automerge action version
Signed-off-by: Nigel Jones <[email protected]>
Unfortunately this isn't working yet. I'm still working with the automerge project to see if it should do what we expect pascalgn/automerge-action#46 |
Have received an update - it seems there are challenges with the way github actions works that means this won't work by default with a fork. Each individual could enable it and we might get something working but it's tedious. I'm going to propose reverting the addition of automerge and delaying this (or closing) until there's time to do further investigation and testing on another repo. |
Signed-off-by: Nigel Jones <[email protected]>
#2453 disable automerge - not able to satisfy original goal
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions. |
still valid |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions. |
Closing this as github now offers this natively https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/automatically-merging-a-pull-request, selectable on a PR basis. Simple but should be more reliable than any of the myriad of plugins |
Currently merging PRs into master can be somewhat tedious.
When a PR is open it is hopefully reviewed, commented on .. and sometime later one of the maintainers sees it, considers it ready to merge, and typically clicks to update from master, then watches it over the next 20-30 minutes until the build completes, then clicks merge.
If other activity is ongoing this may take a few retries.
So we incur a significant amount of time even after that 'intent' is clear.
I would like to propose that we enable an auto-merging bot such as https://github.com/marketplace/actions/merge-pull-requests
Whilst the old workflow will still work, this will allow us to also simply tag the PR to mark our intent, and let the action worry about re-updating and trying the PR merge. For example we would tag the PR as 'automerge'
Upsides - less effort, time,distraction - quicker to clear backlog
Downsides - accidental/liberal use of automerge tag, bugs in process, very easy to queue up a series of PRs with no (easy) way to manage the queue.
Note that 'dependabot' has it's own integrated automerging which is currently enabled. Once a PR is approved it will do a similar action to that above.
There's a few alternative bots to the one above such as https://github.com/marketplace/actions/pr-merge-bot which additionally will enforce that all reviews are in approved state (even if not mandatory by github settings) which adds a little extra safety ? May be an improvement.
interested in the team's thoughts
cc: @mandy-chessell @cmgrote @grahamwallis @cong78
The text was updated successfully, but these errors were encountered: