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

Support GitHubs enable-auto-merge natively #4143

Closed
huehnerlady opened this issue Aug 17, 2021 · 13 comments
Closed

Support GitHubs enable-auto-merge natively #4143

huehnerlady opened this issue Aug 17, 2021 · 13 comments
Labels
T: feature-request Requests for new features

Comments

@huehnerlady
Copy link

Hi, since February the auto-merge function is now generally available.

I have seen here that you can create some github action to set that flag in the dependabot pull requests. But why can't dependabot do that himself?

I would see it as a setting independabot.yml. There should be an option, eg. enable-auto-merge with merge, squash, rebase and disabled as option, disabled being the default.

Please add this functionality to dependabot

@huehnerlady huehnerlady added the T: feature-request Requests for new features label Aug 17, 2021
@huehnerlady huehnerlady changed the title Support enabling of auto-merge natively Support GitHubs enable-auto-merge natively Aug 17, 2021
@lucacome
Copy link

Unfortunately, that action doesn't even work with protected branches.

@asciimike
Copy link
Contributor

asciimike commented Aug 24, 2021

Closing as a dupe of #1973/#2268 (which has a longer discussion and history of this request).

@huehnerlady
Copy link
Author

@asciimike I am confused, for me that is something different. the other one talks about the PRs are merged without a review (at least that is what I understand from the issue). Also as the other issue is open since mid-2020 and the auto-merge feature from Github is added early 2021, I would ask you to please reopen this issue as it is different.

This issue is about the new feature to add auto-merge, so that the PR is merged once it is approved and checks have passed. It is NOT the intention to also skip the approval process

@asciimike
Copy link
Contributor

Dependabot Preview's auto-merge could be set up to respect approval and respected checks as well (see docs), so functionally it's the same feature request, regardless of whether it's "have Dependabot enable GitHub's auto-merge" or "let Dependabot wait for checks and approval and then merge."

IMO, the functionality of "let Dependabot trigger an Action which can approve, merge, etc." is better for developers and Dependabot because it is more flexible with fairly low overhead (if you want to enable auto-merge on certain dependencies, or patch versions, or whatever, you can do that with a pretty limited amount of config, as you've already linked to; if you only want to merge on Tuesdays or when the moon is full or whatever, that's also possible, but it wouldn't be in a dependabot.yml implementation).

@huehnerlady
Copy link
Author

I was just thinking that with the auto-merge there is quite a bit of discussiong around the moral, and so I was hoping to detach this feature request.

It also seems the other ones are more discussing workarounds than having a solution from dependabot, and as I am not interested in workarounds but in a solution from dependabot I was thinking maybe start small?

@asciimike
Copy link
Contributor

We very much appreciate the initiative and pragmatism on trying to make this experience better.

Outside of the question of "if Dependabot should offer auto-merge" there are several concerns I would have about this:

  • What if the user doesn't have auto-merge enabled on the repo? Does Dependabot try to turn it on? Do we fail silently? Where would we even show an error (Dependabot Updates UI, on the code viewer, on the check run linting the file)? We don't have a particularly good way of propagating errors, so I worry this will cause additional confusion.
  • Is this feature going to increase our support burden because of the above, or because of confusion with GitHub's auto-merge vs Dependabot Preview's auto-merge?
  • Is the dependabot.yml really the best place to put this, or do folks want more configurability than we can offer? Is it going to benefit users if we provide something simple that doesn't suit the needs of ~50% of users (on preview, 50% auto-merged everything, but the other 50% only enabled it for certain packages/SEMVER ranges--does this feature have to include all that functionality to be useful)?

In my opinion, delegating this type of functionality to Actions solves these problems: errors are clear in the Actions workflow runs, users are able to configure it as they desire, and there is a more robust community that is able to help debug workflows.

@huehnerlady
Copy link
Author

@asciimike thanks for your answer. I have an action that creates a pull request once a new gradle wrapper version is created. in that you can add a team as a reviewer. For the team to be added it needs to be added in the repo in the first place, so I would argue quite similar to the issue you mentioned. The action does not fail (as the pull request was created), BUT adds a comment to the pull request to explain the problem. Maybe that would be a solution to show the error?
Also I assume there would be documentation added if a feature like that is implemented, so I would say there is a good place to say the conditions for it to work is that it has to be enabled.

All in all I do completely understand your point. This issue would not solve the automatically approving and merging functionality, but I see this feature a bit separate. Maybe the name is the problem and creates so much confusion, especially as the old version of dependabot had that auto approve-merge functionality. But I would see this more as if you would create a functionality where you can add an assignee for example (I had a look and saw this is possible, so thought this might be a good example?). That also is a native GitHub feature you just make use of, instead of actually implementing some functionality (like automatically approving and merging would be).

What do you think?

@asciimike
Copy link
Contributor

Maybe that would be a solution to show the error?
Also I assume there would be documentation added if a feature like that is implemented, so I would say there is a good place to say the conditions for it to work is that it has to be enabled.

I think that these are reasonable options. You'd be surprised by how few people read the fine print of the docs though, so having it in the product experience is always the better option.

But I would see this more as if you would create a functionality where you can add an assignee for example (I had a look and saw this is possible, so thought this might be a good example?). That also is a native GitHub feature you just make use of, instead of actually implementing some functionality (like automatically approving and merging would be).

These features (labeling is another example) do exist, and pre-date the widespread use of Actions. If Actions had existed and was more widely adopted, it's questionable as to whether they would have added them because they are relatively easy to do with an Action. Now that Actions exists and is very popular, I don't see the continued need to shove parameters into the config file unless they truly need to live there (or they are so widely used in the same way in the product that no configuration is needed; we don't think auto-merge is this way).

@huehnerlady
Copy link
Author

Now that Actions exists and is very popular, I don't see the continued need to shove parameters into the config file unless they truly need to live there

This is very interesting. GitHub Actions are very useful for building, triggering updates and also for deployments. But to use actions to configure Pull requests done by a bot feel more like a workaround to me. I do have to check in the action if this pull request actually came from dependabot, not just anybody. In worst case I do have to set credentials for that action and have to maintain it.
I do find a flag in a configuration a lot easier than that. And as dependabot belongs to GitHub for me the feeling occurs that it then should support the basic features of GitHub as well...

@asciimike
Copy link
Contributor

I do find a flag in a configuration a lot easier than that. And as dependabot belongs to GitHub for me the feeling occurs that it then should support the basic features of GitHub as well...

I view GitHub Actions as an all-purpose escape hatch to extend how GitHub products work. We won't include all features in GitHub in Dependabot, so this is the way of allowing folks to configure Dependabot in any number of additional ways we can't think of.

In this instance, regardless of the above, I believe adding an enable-github-automerge key would prove to be confusing to users who expect it to work in the same way as the original Dependabot auto-merge and thus be a support burden (as well as a "well, if you did that, why don't you just re-implement auto-merge" conversation starter). I recognize the desire to have a config option for enabling it, but I don't think it's something we're able to support now or for the foreseeable future given that there's an easy and more configurable workaround.

@huehnerlady
Copy link
Author

ok, thank you for your reasoning and your answers

@zteater
Copy link

zteater commented Aug 29, 2021

@asciimike I agree with @huehnerlady's perspective, but disagree in their approach.

From my perspective, Dependabot should not be circumventing how my repository is setup. When Dependabot raises a PR, the PR should be treated as no different than if a human had raised it. If I have enabled automatic merging, then I've already accepted the risks and it should flow through just as any other PR.

@asciimike
Copy link
Contributor

From my perspective, Dependabot should not be circumventing how my repository is setup. When Dependabot raises a PR, the PR should be treated as no different than if a human had raised it. If I have enabled automatic merging, then I've already accepted the risks and it should flow through just as any other PR.

I agree and don't plan to change it: if you have GitHub auto-merge enabled, Dependabot PRs will respect those settings and be merged accordingly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: feature-request Requests for new features
Projects
None yet
Development

No branches or pull requests

4 participants