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

Reusing logic from kubernetes/test-infra? #61

Open
carolynvs opened this issue Jun 5, 2022 · 7 comments
Open

Reusing logic from kubernetes/test-infra? #61

carolynvs opened this issue Jun 5, 2022 · 7 comments

Comments

@carolynvs
Copy link
Contributor

Some things in prow are pretty straightforward to re-implement, but some is quite a bit of logic. A big feature that I'm interested in is determining which reviewers and approvers to select for a pull request when there can be multiple OWNERS files defined in a repository, and then ensuring that a PR isn't merged until approvers from all OWNERS files have given approval. So if a PR touches two directories, which have different approvers, both approvers would have to approve the PR before it is merged by our github action. (Correct me if I'm misunderstanding how prow works!)

The kuberentes/test-infra repository defines this logic and has functions that we could export, wrap and call from javascript. Alternatively we could port these functions to typescript.

Before I spent time doing any of that, I wanted to get a feel for what you would prefer? I'm down for implementing either solution.

@jpmcb
Copy link
Owner

jpmcb commented Jul 22, 2022

@carolynvs - Hmmmm. I'm not sure; I had looked at re-using some Go code from prow and test-infra early on, but at the time, the Go octokit API library was being archived. But seems there is a google maintained go-github library nowadays that may work for our needs.

I'm also not sure what real benefit we would get from a full re-write vs a port. I'd consider this github action fairly stable and I worry about introducing more complexity for users and maintainers if we wanted to re-using much logic from kubernetes in Go.

Generally, I'm not a fan of re-writing this action in Go. I like having nock and jest for API mocking and in my experience, that's painful in Go.

But it sounds like you're asking if we could look into somehow importing that Go code and wrapping it around executors we could use in this javascript action? I'd be all for investigating that and if it's stable, let's do it!

Opinions welcome!

@jpmcb
Copy link
Owner

jpmcb commented Jul 22, 2022

Maybe a simple example of how we could do that with a small go library would help me wrap my head around what you're suggesting. If I had to read between the lines, sounds like how you'd do that is

  • import the necessary libraries with public methods into our own go program
  • build/compile that binary
  • have our action (written in TS) execute that binary with some inputs, deriving the outputs

?

@carolynvs
Copy link
Contributor Author

Oh yeah sorry, I totally am not interesting in reimplementing the gh-action in Go! 😬 What you suggested above was exactly what I was thinking for how to reuse test-infra without having to port chunks of it from go to typescript. I'm not sure which is more effort to maintain over time so I may explore both options.

I mostly wanted to make sure that if one was super distasteful that I knew not to waste time going down that path.

@jpmcb
Copy link
Owner

jpmcb commented Jul 22, 2022

I'm super into this investigation! Absolutely happy to chime in as well!

@ashleypafko
Copy link

Hey @carolynvs & @jpmcb ! I've been keeping an eye on this issue and pretty interested in this feature... just curious, are you guys currently working on this / is it close to being done? Love the project 🙂

@jpmcb
Copy link
Owner

jpmcb commented Sep 27, 2022

Hi @ashleypafko - I personally haven't been investigating this (I wish I had more time to do open source stuff 😭)

I'm open to any contributions on this and I'd be super interested to hear what could work. If you want to pick this up, I'd be happy to see what you come up with!

@carolynvs
Copy link
Contributor Author

Same, I haven't had time to work on this but would be super interested in seeing it implemented!

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