ideas
ideas copied to clipboard
Merge pull requests when all status checks are passing
For some projects (with really good test coverage?) it would be cool to have a bot that automatically merges any PR that has passing status checks, e.g. tests passing, reviews approved, etc.
If this existed, we would probably use it on the Electron website: https://github.com/electron/electronjs.org/issues/1046
@gr2m suggested that there might currently be some roadblocks for such a bot: https://platform.github.community/t/repositories-which-have-protected-branches-with-push-restrictions-have-no-ability-to-grant-push-rights-to-integrations/1376
we're thinking about doing something similar for Angular, we already have a probot app that checks if the PR is good to merge, but it doesn't do the merge itself yet
Hey @ocombe is that bot source public?
yes! it's here: https://github.com/angular/github-robot we're going to add some new PR triage functionalities soon as well, see https://github.com/angular/angular/issues/21884 for a first draft of what we intend to do
GitHub
Contribute to github-robot development by creating an account on GitHub.
Good stuff. Thanks for sharing, @ocombe.
I seen https://github.com/angular/angular/issues/21884. I know to work with github api's but no working experience for bot in github.
I would like to work on this any suggestions on how the bot should behave if the mentioned issue where bots not being able to merge pull requests with protected branches
So I went through this https://platform.github.community/t/repositories-which-have-protected-branches-with-push-restrictions-have-no-ability-to-grant-push-rights-to-integrations/1376
.
And it doesn't look like GitHub has released a fix yet.
Further AFAIK there is no realistic workaround for this?
Can anyone give some idea so as to how we might go about this?
I don't think that an app should be able to bypass protected branches. Those are typically very sensitive branches where things would break if merged incorrectly, and I think that folks would be upset to see an app ignore their intentional decision to treat that branch more carefully.
Protected branches refer to branches with some restriction on who can push to the branch. Isn't that the case for most branches?
@JasonEtco it’s not about working around protected branches, it's about automated merging once all the checks are green
it’s not about working around protected branches, it's about automated merging once all the checks are green
I see - I guess it depends on the level of protection (which is ultimately up to the user). One available option is the only allow merges with at least 1 green review from someone with write access; I think that as long as that protection isn't ignored then its all good. I only bring it up because I'm not sure if that's considered a "status."
Sadly the GitHub API doesn't support GitHub apps merging pull requests with protected branches yet. @JasonEtco
GitHub API doesn't support GitHub apps merging pull requests with protected branches yet.
I found the internal GitHub issue for this and gave it a little bump. It's a known issue, GitHubbers are aware that integrators are frustrated by it, and a solution is actively being discussed. I'll report back here with any updates.
@zeke That's awesome.Thanks
@zeke I would like to work on this. I have completed hello-world bot. What's the next step?
@pranay414 GitHub folks are actively working on a fix for the bot permissions problem, but it hasn't shipped yet. I will post here when that happens.
@zeke so should I start drafting a proposal for this?
@pranay414 sure. If you want to outline the expected behavior that could be helpful.
@zeke when someone submits a PR then it is put into testing and once all checks are passed then it should be merged or it should be reviewed by someone and then merged?
@pranay414 One of the status checks will probably be getting the PR reviewed.
@rsarky so once it is reviewed then it should be merged right?
@pranay414 Once all the status checks pass. The status checks may include CI, reviews etc.
Ok got it, Thanks :)
@gr2m @JasonEtco I would like to consider this for my GSoC proposal. Can you please guide me how do I proceed?
@gr2m @JasonEtco . I have given this task a try and it works for ci-validated and mergeable PRs. Please find my repository here gsoc_probot_task_1
@zeke A new API endpoint is now available and it is perfect for this use case :tada: Merge PR
@rsarky you beat me to it! Thanks to @kytrinyx for helping move this forward.
https://twitter.com/kytrinyx/status/986409138568810496
i'm not sure how helpful any of this is to this thread, but i figure its related enough to at least mention. i maintain greenkeeper-keeper that serves a very similar purpose, but is purposely limited to PRs that are from greenkeeper. the current implementation is a hapi plugin, but a probot version is on my list even though i haven't found time to make that happen yet. i'm very interested in where this goes as i could see potentially replacing greenkeeper-keeper
with this.
one thing that might be worth considering is the ability to limit the automatic merge to a certain type of PR. the important detail for greenkeeper-keeper
is that it is limited to only greenkeeper PRs. they are only dependency upgrades, so there isnt anything to review beyond passing commit checks. contributions from humans usually are more more worth a human review, so my team likes to handle those differently.
I almost never want to merge pull requests unseen as soon as they pass—it's almost always a thing where I've reviewed it, and want to merge it, but the build is still running, so I go look at instagram and check my email and completely forget what I was actually doing. That's when I would want to have a way to signal to a bot: go ahead and merge this as soon as it passes.
I could see this working with protected branches that have required builds and require an approval from a human reviewer. Then as soon as all the requirements are met, it could merge it.