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

Auto merge support for GitHub #3368

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mberndt123
Copy link

@mberndt123 mberndt123 commented May 30, 2024

Hey there,

I've hacked together something for #1960: Auto-merge support on Github, not really to get it merged in its current form but to get the discussion started.

The issue here is that auto-merge is not available through the Github REST API but only through the GraphQL one.

Here are some questions that need answers:

  • do we need to migrate GitHubApiAlg as a whole to use the GraphQL API, or do we just mix REST and GraphQL calls?
  • this PR is based on code generated from the GraphQL Github API spec. Is this the route we want to take, or do we want to do something else?
  • is it OK to check in their spec into version control? I'd rather have avoided this, but it crashes when I tell the caliban sbt plugin to download the spec, so I checked it in for now

Needless to say, I haven't tried running this and it doesn't really do error handling either.

@987Nabil
Copy link
Contributor

Auto merge can be enabled via GitHub CLI. That is actually easy and preinstalled on GH runners

@alejandrohdezma
Copy link
Member

Hey @mberndt123, thank you for this. I was going to suggest the same thing as @987Nabil (you can see it in action here). What would be your usecase for this instead of just add that at the workflow level?

@mberndt123
Copy link
Author

mberndt123 commented May 30, 2024

Thanks @alejandrohdezma ,

Well I might not want to enable it indiscriminately but only for the PRs created by scala-steward. It can be done, but I also don't want to adapt the CI configuration for all of the dozens of repositories that I'm working on. It's more convenient to just add a command line flag in the scala-steward configuration.

I also think there's ample precedent for this as the equivalent Gitlab option is already supported.

@alejandrohdezma
Copy link
Member

Yeah, that makes sense. In that case I think it would be simpler to just make the call using the HTTP client than adding Caliban and the GitHub GraphQL definition just for one enpdoint.

@987Nabil
Copy link
Contributor

987Nabil commented Jun 4, 2024

I think using gh cli is preferable. If the underlying call ever changes, the CLI call would probably still look the same.

@alejandrohdezma
Copy link
Member

Yeah, I agree on that but I don't think we can assume Scala Steward will always run on GH actions

@987Nabil
Copy link
Contributor

@alejandrohdezma I think it is okay to assume, that if you open GH prs you run on a machine that has the cli. But adding a little check, if it is there and if not log something like You need to have gh cli installed is probably okay.
I would assume that 99% of runners have it. I would implement it via cli, document that github cli is needed and preinstalled on gh runners and see if users complain. The implementation could be adjusted if really needed

@alejandrohdezma
Copy link
Member

@scala-steward-org/core any further opinions on adding a dependency with an external CLI?

@mzuehlke
Copy link
Member

If we propperly documentz that this feature requires the CLI and if the absence of the CLI doesn't break anything it is fine with me.
We for example are running on custom runners and also Githuib recently removed tools (sbt) from their runner.

@mberndt123
Copy link
Author

I would argue against using the Github CLI for this. As far as I can see, it's a single request to a very simple endpoint, which shouldn't be hard to implement using just Scala and an HTTP client. It's just that I'm too busy to work on this right now.
Using the Github CLI would imply adding that to the Docker image too, which is yet more hassle.

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

Successfully merging this pull request may close these issues.

4 participants