Implement support for actions workflow jobs #1421

joshuabezaleel · 2020-02-10T14:04:34Z

This PR adds support for managing the most recent update of Action Workflow Jobs.

Relates to: #1399

By the way, there are several things that I am still not sure of:

The action in the original GitHub documentation is "List workflow job logs" thus I name the function ListWorkflowJobLogs but since it basically retrieve a URL to download a text file that contain the logs I think it would be better to name it GetLogsFileURL.
I am still not sure of perhaps how go-github would implement this particular function since the URL of the file is in the header of the response. Do you have any example of other functions that implement this kind of response?

Thank you very much beforehand 🙂

joshuabezaleel · 2020-02-10T14:12:11Z

Apparently I pasted the wrong test, I am sorry.

gmlewis · 2020-02-10T14:15:38Z

Thanks, @joshuabezaleel!

By the way, there are several things that I am still not sure of:

Yes, I think this one should probably be treated like we treat GetArchiveLink where the client can either get the URL or get the actual text file using the followRedirects bool parameter.

Does that make sense?

joshuabezaleel · 2020-02-11T01:59:16Z

Yes, the function that you mentioned was really helpful, thank you very much @gmlewis !! Looking forward to your review 🙂

gmlewis

Thank you, @joshuabezaleel!

One tiny nit, otherwise LGTM.

Were you able to try this out to see if it worked?
I ask because I know we recently added a different followRedirectsClient mechanism to RepositoriesService.DownloadReleaseAsset and am not sure which is the better implementation, actually.

If it turns out you try this one and it doesn't work, then maybe we could try the other mechanism which I know was tested and might be a better solution.

Either way, awaiting second LGTM before merging.

gmlewis · 2020-02-11T02:46:40Z

github/actions_workflow_jobs.go

+	return job, resp, nil
+}
+
+// GetWorkflowJobLogs gets a redirect URL to a download a plain text file of logs for a workflow job.


nit: s/to a download/to download/

joshuabezaleel · 2020-02-11T05:12:20Z

I tried to use the ActionsService from a constructed client to try it but I couldn't find it. Do you have any idea why? .... 😕

marcofranssen · 2020-02-11T08:40:35Z

@joshuabezaleel

probably you need to do

go get github.com/google/go-github@thebranch-containing-the-code

or you should add a replace in your go.mod

replace github.com/google/go-github => ./the-path-to-your-checkout-of-the-repo-branch

joshuabezaleel · 2020-02-11T09:49:54Z

@marcofranssen The main.go that I wrote to try the ActionsService is in the go-github folder itself. And I already fetch the latest commits that contains the updates of including ActionsService when initiating a new Client so that should already be included though. 😕

gmlewis · 2020-02-11T12:42:11Z

Can you please start with a super-simple example like this: #1176 (comment)
and tweak it to call client.Actions.YourMethodYouWantToTest and share it here, and we can help you debug it?

I'm betting that you are trying to call client.ActionsService instead of client.Actions and that is the problem.

gmlewis · 2020-02-12T02:13:22Z

github/actions_workflow_jobs.go

+}
+
+// Job represents a repository action workflow job.
+type Job struct {


The more I think about this, the more I'm thinking that all fields in Job and Step should be pointers and add the ,omitempty tag, just for consistency with the rest of the repo.

I'd like to hear what @gauntface thinks about this, specifically.

Technically, I don't think they are needed... but it might be an odd experience for a user of this repo to find this pair of structs not follow the pattern set by the rest of the repo.

gmlewis · 2020-02-12T02:14:33Z

github/actions_workflow_jobs.go

+
+// Jobs represents a slice of repository action workflow job.
+type Jobs struct {
+	TotalCount int    `json:"total_count"`


Same here:

TotalCount *int `json:"total_count,omitempty"`

gmlewis · 2020-02-12T02:15:34Z

github/actions_workflow_jobs.go

+	StartedAt   Timestamp `json:"started_at"`
+	CompletedAt Timestamp `json:"completed_at"`
+	Name        string    `json:"name"`
+	Steps       []*Step   `json:"steps"`


Steps would be the only exception and is fine as it is currently written, being a slice of pointers.

gmlewis · 2020-02-13T14:27:14Z

github/actions_workflow_jobs.go

+)
+
+// Step represents a single task from a sequence of tasks of a job.
+type Step struct {


In light of #1429, #79, #45, and #19, I'm going to make the executive decision and say that these two structs (Job and Step) should all use pointers.

In addition, please rename Step to be TaskStep to make it more clear when seeing it by itself.

And let's please change Job to WorkflowJob to make it more descriptive as well.

Thank you!

Friendly ping, @joshuabezaleel to switch these fields to pointers, please.

gmlewis · 2020-02-27T01:21:55Z

Thank you, @joshuabezaleel.
Please run go generate ./... again and then the tests should pass.

joshuabezaleel · 2020-02-27T01:58:32Z

Hi @gmlewis , I ran the go generate ./... command but I think it still yield error on the go test and go vet command 😕

gmlewis

~~I believe that if you accept my two suggestions, that the tests should be fixed.~~

I'm going to go ahead and try it out.

github/actions_workflow_jobs_test.go

gmlewis

Those commits fixed it. LGTM.

@wesleimp or @martinssipenko - if you could approve please, we can go ahead and merge this.

martinssipenko

LGTM

joshuabezaleel · 2020-02-27T04:24:07Z

Truly sorry for the hassle and thank you for fixing it up for me, @gmlewis ! Truly appreciate it 🙂

gmlewis · 2020-02-27T12:43:43Z

Truly sorry for the hassle and thank you for fixing it up for me, @gmlewis ! Truly appreciate it slightly_smiling_face

No problem, @joshuabezaleel - you did a great job, and it was my fault for waffling on the pointers. 😄

Thank you, @martinssipenko!
Merging.

Relates to: google#1399.

Implement support for actions workflow jobs

c5b7b72

googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Feb 10, 2020

Fix test for actions_workflow_jobs

cae90e1

Update ListWorkflowJobLogs to GetWorkflowJobLogs

c59a6f8

Update the function name to GetWorkflowJobLogs

22cf2d2

gmlewis approved these changes Feb 11, 2020

View reviewed changes

gmlewis requested a review from gauntface February 11, 2020 02:55

Fix typo

ada4acf

gmlewis reviewed Feb 12, 2020

View reviewed changes

gmlewis mentioned this pull request Feb 12, 2020

Support GitHub Actions API #1399

Closed

gmlewis requested changes Feb 13, 2020

View reviewed changes

joshuabezaleel added 4 commits February 26, 2020 20:51

Convert Task and Job struct's properties to pointer

b695b0a

Add tag omitempty for Task and Job struct's properties

db076ae

ADd tag omitempty to Jobs struct properties

518cd9d

Rename struct Task to TaskStep and Job to WorkflowJob

1bae46c

Running go generate command

016438d

gmlewis requested changes Feb 27, 2020

View reviewed changes

github/actions_workflow_jobs_test.go Outdated Show resolved Hide resolved

github/actions_workflow_jobs_test.go Outdated Show resolved Hide resolved

gmlewis added 2 commits February 26, 2020 21:16

Update github/actions_workflow_jobs_test.go

00b8d43

Update github/actions_workflow_jobs_test.go

aefa9a0

Merge branch 'master' into actions-workflow-jobs

ac8c3cf

gmlewis approved these changes Feb 27, 2020

View reviewed changes

martinssipenko approved these changes Feb 27, 2020

View reviewed changes

gmlewis merged commit 300f887 into google:master Feb 27, 2020

n1lesh pushed a commit to n1lesh/go-github that referenced this pull request Oct 2, 2020

Implement support for actions workflow jobs (google#1421)

866fffe

Relates to: google#1399.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement support for actions workflow jobs #1421

Implement support for actions workflow jobs #1421

joshuabezaleel commented Feb 10, 2020

joshuabezaleel commented Feb 10, 2020

gmlewis commented Feb 10, 2020

joshuabezaleel commented Feb 11, 2020

gmlewis left a comment

gmlewis Feb 11, 2020

joshuabezaleel commented Feb 11, 2020

marcofranssen commented Feb 11, 2020

joshuabezaleel commented Feb 11, 2020

gmlewis commented Feb 11, 2020

gmlewis Feb 12, 2020

gmlewis Feb 12, 2020

gmlewis Feb 12, 2020

gmlewis Feb 13, 2020

gmlewis Feb 22, 2020

gmlewis commented Feb 27, 2020

joshuabezaleel commented Feb 27, 2020

gmlewis left a comment •

edited

Loading

gmlewis left a comment

martinssipenko left a comment

joshuabezaleel commented Feb 27, 2020

gmlewis commented Feb 27, 2020

Implement support for actions workflow jobs #1421

Implement support for actions workflow jobs #1421

Conversation

joshuabezaleel commented Feb 10, 2020

joshuabezaleel commented Feb 10, 2020

gmlewis commented Feb 10, 2020

joshuabezaleel commented Feb 11, 2020

gmlewis left a comment

Choose a reason for hiding this comment

gmlewis Feb 11, 2020

Choose a reason for hiding this comment

joshuabezaleel commented Feb 11, 2020

marcofranssen commented Feb 11, 2020

joshuabezaleel commented Feb 11, 2020

gmlewis commented Feb 11, 2020

gmlewis Feb 12, 2020

Choose a reason for hiding this comment

gmlewis Feb 12, 2020

Choose a reason for hiding this comment

gmlewis Feb 12, 2020

Choose a reason for hiding this comment

gmlewis Feb 13, 2020

Choose a reason for hiding this comment

gmlewis Feb 22, 2020

Choose a reason for hiding this comment

gmlewis commented Feb 27, 2020

joshuabezaleel commented Feb 27, 2020

gmlewis left a comment • edited Loading

Choose a reason for hiding this comment

gmlewis left a comment

Choose a reason for hiding this comment

martinssipenko left a comment

Choose a reason for hiding this comment

joshuabezaleel commented Feb 27, 2020

gmlewis commented Feb 27, 2020

gmlewis left a comment •

edited

Loading