Implement support for actions runners #1446
Conversation
googlebot
added
the
cla: yes
There was a problem hiding this comment.
This is looking great, @nightlark!
Just a few minor tweaks, please, and then I think we will be ready for a second review and merging.
github/actions_runners.go
Outdated
| Filename *string `json:"filename,omitempty"` | ||
| } | ||
|
|
||
| // ListRunnerApplicationDownloads lists self-hosted runner applicatio binaries that can be downloaded and run. |
github/actions_runners.go
Outdated
| // ListRunnerApplicationDownloads lists self-hosted runner applicatio binaries that can be downloaded and run. | ||
| // | ||
| // GitHub API docs: https://developer.github.com/v3/actions/self_hosted_runners/#list-downloads-for-the-self-hosted-runner-application | ||
| func (s *ActionsService) ListRunnerApplicationDownloads(ctx context.Context, owner, repo string) (*[]RunnerApplicationDownload, *Response, error) { |
There was a problem hiding this comment.
s/*[]Runner.../[]*Runner.../
github/actions_runners.go
Outdated
| return nil, nil, err | ||
| } | ||
|
|
||
| runnerApplicationDownloads := new([]RunnerApplicationDownload) |
There was a problem hiding this comment.
var rads []*RunnerApplicationDownload
resp, err := s.client.Do(ctx, req, &rads)
|
|
||
| // ListRunners lists all the self-hosted runners for a repository. | ||
| // | ||
| // GitHub API docs: https://developer.github.com/v3/actions/self_hosted_runners/#list-self-hosted-runners-for-a-repository |
There was a problem hiding this comment.
Can you please contact [email protected] and ask them why their example shows a list-of-lists here:
https://developer.github.com/v3/actions/self_hosted_runners/#list-self-hosted-runners-for-a-repository
(and ask them to please fix it if it truly is a single list, which I think it should be)? Thank you!
There was a problem hiding this comment.
That’s the bit in the description where I mentioned contacting support about the array nested two levels deep (the real endpoint response doesn’t match their example). I'm not sure if they'll respond until the week starts.
There was a problem hiding this comment.
Whups, I have a bad habit of looking at the code before I look at the PR description.
Sorry about that, and thank you!
😁
github/actions_runners.go
Outdated
| // ListRunners lists all the self-hosted runners for a repository. | ||
| // | ||
| // GitHub API docs: https://developer.github.com/v3/actions/self_hosted_runners/#list-self-hosted-runners-for-a-repository | ||
| func (s *ActionsService) ListRunners(ctx context.Context, owner, repo string, opts *ListOptions) (*[]Runner, *Response, error) { |
github/actions_runners.go
Outdated
| return nil, nil, err | ||
| } | ||
|
|
||
| runners := new([]Runner) |
There was a problem hiding this comment.
var runners []*Runner
resp, err := s.client.Do(ctx, req, &runners)
|
Alright, suggested changes are made! |
There was a problem hiding this comment.
Thank you, @nightlark !
LGTM. Awaiting second LGTM before merging.
|
Thank you, @wesleimp! |
|
Here’s the response from GitHub support on the list runners example: |
|
Thank you, @nightlark! As I said in another PR (heh, I think a lot of their reports come from volunteers working on this repo... which is why I delegate to avoid the "Oh, not gmlewis again!" 😄) reporting the errors in the official GitHub v3 API doc just helps to make a better experience for all users of GitHub, which is fantastic. |
This PR adds support for the GitHub Actions Runners API.
Relates to: #1399
For the list self hosted runners endpoint I sent an email to support regarding the example showing the array nested two levels deep which doesn't match the actual responses I was getting, and inquiring about if they intended to include a
total_countfield like most other endpoints that support pagination.One of the things I'm not sure about is the return and struct field types, specifically which ones should be references (e.g.
*[]Typeor*[]*Typefor a list return type, or something else?)