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

Add Readme for prow/github #12760

Closed
chases2 opened this issue May 23, 2019 · 6 comments
Closed

Add Readme for prow/github #12760

chases2 opened this issue May 23, 2019 · 6 comments
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@chases2
Copy link
Contributor

chases2 commented May 23, 2019

What should be cleaned up or changed:
test-infra/prow/github/ appears to be a useful library for reading & modifying GitHub programmatically using the v4 API. It would be great to use wherever we're doing GitHub Things, but it's got some very large files.

A README.md in this directory would help a lot.

Provide any links for context:
https://github.com/kubernetes/test-infra/tree/master/prow/github

@chases2 chases2 added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label May 23, 2019
@stevekuznetsov
Copy link
Contributor

@chases2 are you hoping to use it inside of prow our in other projects? The directory vendors a v4 (graphql) client and implements a v3 (REST) client, but exposes them in opinionated ways that make sense for Prow but maybe not other projects. Fields Prow does not use, for instance, are elided.

@chases2
Copy link
Contributor Author

chases2 commented May 24, 2019

I was looking at using it for a TestGrid feature (see Issue #10701 and PR #12753), because it might be duplicating the functionality I've written in that PR.

The thing is, it's tough to tell whether or not it meets those needs, or exactly what it's doing with GitHub's API. I've been parsing through it, but it's thousands of lines. It would be real neat if the next new person didn't have to.

In addition, a note on its purpose ("This is suitable for re-use" or "This is really just for Prow, and maybe here's why") would give some insight that code can't.

@stevekuznetsov
Copy link
Contributor

Inside of this repo it's definitely suitable for re-use since it will be compile-time enforced that refactors won't break you. Right now the biggest risks with using it are:

  • if Prow needs it to change, it will change
  • when Prow does not need fields on objects that GitHub sends, it does not deserialize them and they are therefore ignored

If you're in the repo, the first issue is nullified since you will be refactored as well to continue to compile. If you hit the second issue, please just feel free to add the fields you need to the objects we have.

chases2 added a commit to chases2/test-infra that referenced this issue Jun 7, 2019
A short description of the client to encourage reuse
Addresses kubernetes#12760
@chases2
Copy link
Contributor Author

chases2 commented Jun 7, 2019

/assign

chases2 added a commit to chases2/test-infra that referenced this issue Jun 10, 2019
A short description of the client to encourage reuse
Addresses kubernetes#12760
@chases2
Copy link
Contributor Author

chases2 commented Jun 10, 2019

Fix has been merged; closing
/close

@k8s-ci-robot
Copy link
Contributor

@chases2: Closing this issue.

In response to this:

Fix has been merged; closing
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mirandachrist pushed a commit to mirandachrist/test-infra that referenced this issue Jun 17, 2019
A short description of the client to encourage reuse
Addresses kubernetes#12760
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

No branches or pull requests

3 participants