Clean up method return consistency

github/actions_artifacts.go

@@ -110,43 +110,17 @@ func (s *ActionsService) GetArtifact(ctx context.Context, owner, repo string, ar
 func (s *ActionsService) DownloadArtifact(ctx context.Context, owner, repo string, artifactID int64, followRedirects bool) (*url.URL, *Response, error) {
 	u := fmt.Sprintf("repos/%v/%v/actions/artifacts/%v/zip", owner, repo, artifactID)
 
-	resp, err := s.getDownloadArtifactFromURL(ctx, u, followRedirects)
+	resp, err := s.client.getDownloadArtifactFromURL(ctx, u, followRedirects)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	if resp.StatusCode != http.StatusFound {
 		return nil, newResponse(resp), fmt.Errorf("unexpected status code: %s", resp.Status)
 	}
-	parsedURL, err := url.Parse(resp.Header.Get("Location"))
-	return parsedURL, newResponse(resp), nil
-}
-
-func (s *ActionsService) getDownloadArtifactFromURL(ctx context.Context, u string, followRedirects bool) (*http.Response, error) {
-	req, err := s.client.NewRequest("GET", u, nil)
-	if err != nil {
-		return nil, err
-	}
 
-	var resp *http.Response
-	// Use http.DefaultTransport if no custom Transport is configured
-	req = withContext(ctx, req)
-	if s.client.client.Transport == nil {
-		resp, err = http.DefaultTransport.RoundTrip(req)
-	} else {
-		resp, err = s.client.client.Transport.RoundTrip(req)
-	}
-	if err != nil {
-		return nil, err
-	}
-	resp.Body.Close()
-
-	// If redirect response is returned, follow it
-	if followRedirects && resp.StatusCode == http.StatusMovedPermanently {
-		u = resp.Header.Get("Location")
-		resp, err = s.getDownloadArtifactFromURL(ctx, u, false)
-	}
-	return resp, err
+	parsedURL, err := url.Parse(resp.Header.Get("Location"))
+	return parsedURL, newResponse(resp), err
 }
 
 // DeleteArtifact deletes a workflow run artifact.

github/actions_artifacts_test.go

@@ -25,7 +25,7 @@ func TestActionsService_ListArtifacts(t *testing.T) {
 		testFormValues(t, r, values{"page": "2"})
 		fmt.Fprint(w,
 			`{
-				"total_count":1, 
+				"total_count":1,
 				"artifacts":[{"id":1}]
 			}`,
 		)
@@ -107,7 +107,7 @@ func TestActionsService_ListWorkflowRunArtifacts(t *testing.T) {
 		testFormValues(t, r, values{"page": "2"})
 		fmt.Fprint(w,
 			`{
-				"total_count":1, 
+				"total_count":1,
 				"artifacts":[{"id":1}]
 			}`,
 		)

github/actions_workflow_jobs.go

@@ -114,41 +114,16 @@ func (s *ActionsService) GetWorkflowJobByID(ctx context.Context, owner, repo str
 func (s *ActionsService) GetWorkflowJobLogs(ctx context.Context, owner, repo string, jobID int64, followRedirects bool) (*url.URL, *Response, error) {
 	u := fmt.Sprintf("repos/%v/%v/actions/jobs/%v/logs", owner, repo, jobID)
 
-	resp, err := s.getWorkflowLogsFromURL(ctx, u, followRedirects)
+	// The DownloadArtifact in this case are the workflow logs URL.
+	resp, err := s.client.getDownloadArtifactFromURL(ctx, u, followRedirects)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	if resp.StatusCode != http.StatusFound {
 		return nil, newResponse(resp), fmt.Errorf("unexpected status code: %s", resp.Status)
 	}
+
 	parsedURL, err := url.Parse(resp.Header.Get("Location"))
 	return parsedURL, newResponse(resp), err
 }
-
-func (s *ActionsService) getWorkflowLogsFromURL(ctx context.Context, u string, followRedirects bool) (*http.Response, error) {
-	req, err := s.client.NewRequest("GET", u, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	var resp *http.Response
-	// Use http.DefaultTransport if no custom Transport is configured
-	req = withContext(ctx, req)
-	if s.client.client.Transport == nil {
-		resp, err = http.DefaultTransport.RoundTrip(req)
-	} else {
-		resp, err = s.client.client.Transport.RoundTrip(req)
-	}
-	if err != nil {
-		return nil, err
-	}
-	resp.Body.Close()
-
-	// If redirect response is returned, follow it
-	if followRedirects && resp.StatusCode == http.StatusMovedPermanently {
-		u = resp.Header.Get("Location")
-		resp, err = s.getWorkflowLogsFromURL(ctx, u, false)
-	}
-	return resp, err
-}

github/actions_workflow_runs.go

@@ -231,14 +231,16 @@ func (s *ActionsService) CancelWorkflowRunByID(ctx context.Context, owner, repo
 func (s *ActionsService) GetWorkflowRunLogs(ctx context.Context, owner, repo string, runID int64, followRedirects bool) (*url.URL, *Response, error) {
 	u := fmt.Sprintf("repos/%v/%v/actions/runs/%v/logs", owner, repo, runID)
 
-	resp, err := s.getWorkflowLogsFromURL(ctx, u, followRedirects)
+	// The DownloadArtifact in this case are the workflow logs.
+	resp, err := s.client.getDownloadArtifactFromURL(ctx, u, followRedirects)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	if resp.StatusCode != http.StatusFound {
 		return nil, newResponse(resp), fmt.Errorf("unexpected status code: %s", resp.Status)
 	}
+
 	parsedURL, err := url.Parse(resp.Header.Get("Location"))
 	return parsedURL, newResponse(resp), err
 }

github/activity_notifications_test.go

@@ -59,7 +59,7 @@ func TestActivityService_ListNotification(t *testing.T) {
 	})
 }
 
-func TestActivityService_ListRepositoryNotification(t *testing.T) {
+func TestActivityService_ListRepositoryNotifications(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
@@ -81,7 +81,7 @@ func TestActivityService_ListRepositoryNotification(t *testing.T) {
 
 	const methodName = "ListRepositoryNotifications"
 	testBadOptions(t, methodName, func() (err error) {
-		_, _, err = client.Activity.ListRepositoryNotifications(ctx, "\n", "\n", nil)
+		_, _, err = client.Activity.ListRepositoryNotifications(ctx, "\n", "\n", &NotificationListOptions{})
 		return err
 	})
 

github/activity_star.go

@@ -108,9 +108,14 @@ func (s *ActivityService) IsStarred(ctx context.Context, owner, repo string) (bo
 	if err != nil {
 		return false, nil, err
 	}
+
 	resp, err := s.client.Do(ctx, req, nil)
 	starred, err := parseBoolResponse(err)
-	return starred, resp, err
+	if err != nil {
+		return false, resp, err
+	}
+
+	return starred, resp, nil
 }
 
 // Star a repository as the authenticated user.
@@ -122,6 +127,7 @@ func (s *ActivityService) Star(ctx context.Context, owner, repo string) (*Respon
 	if err != nil {
 		return nil, err
 	}
+
 	return s.client.Do(ctx, req, nil)
 }
 
@@ -134,5 +140,6 @@ func (s *ActivityService) Unstar(ctx context.Context, owner, repo string) (*Resp
 	if err != nil {
 		return nil, err
 	}
+
 	return s.client.Do(ctx, req, nil)
 }

github/billing.go

@@ -79,7 +79,7 @@ func (s *BillingService) GetActionsBillingOrg(ctx context.Context, org string) (
 		return nil, resp, err
 	}
 
-	return actionsOrgBilling, resp, err
+	return actionsOrgBilling, resp, nil
 }
 
 // GetPackagesBillingOrg returns the free and paid storage used for GitHub Packages in gigabytes for an Org.
@@ -98,7 +98,7 @@ func (s *BillingService) GetPackagesBillingOrg(ctx context.Context, org string)
 		return nil, resp, err
 	}
 
-	return packagesOrgBilling, resp, err
+	return packagesOrgBilling, resp, nil
 }
 
 // GetStorageBillingOrg returns the estimated paid and estimated total storage used for GitHub Actions
@@ -118,7 +118,7 @@ func (s *BillingService) GetStorageBillingOrg(ctx context.Context, org string) (
 		return nil, resp, err
 	}
 
-	return storageOrgBilling, resp, err
+	return storageOrgBilling, resp, nil
 }
 
 // GetAdvancedSecurityActiveCommittersOrg returns the GitHub Advanced Security active committers for an organization per repository.
@@ -137,7 +137,7 @@ func (s *BillingService) GetAdvancedSecurityActiveCommittersOrg(ctx context.Cont
 		return nil, resp, err
 	}
 
-	return activeOrgCommitters, resp, err
+	return activeOrgCommitters, resp, nil
 }
 
 // GetActionsBillingUser returns the summary of the free and paid GitHub Actions minutes used for a user.
@@ -156,7 +156,7 @@ func (s *BillingService) GetActionsBillingUser(ctx context.Context, user string)
 		return nil, resp, err
 	}
 
-	return actionsUserBilling, resp, err
+	return actionsUserBilling, resp, nil
 }
 
 // GetPackagesBillingUser returns the free and paid storage used for GitHub Packages in gigabytes for a user.
@@ -175,7 +175,7 @@ func (s *BillingService) GetPackagesBillingUser(ctx context.Context, user string
 		return nil, resp, err
 	}
 
-	return packagesUserBilling, resp, err
+	return packagesUserBilling, resp, nil
 }
 
 // GetStorageBillingUser returns the estimated paid and estimated total storage used for GitHub Actions
@@ -195,5 +195,5 @@ func (s *BillingService) GetStorageBillingUser(ctx context.Context, user string)
 		return nil, resp, err
 	}
 
-	return storageUserBilling, resp, err
+	return storageUserBilling, resp, nil
 }

github/gen-stringify-test.go

@@ -68,6 +68,12 @@ var (
 				return "github.Timestamp{0001-01-01 00:00:00 +0000 UTC}"
 			case "nil":
 				return "map[]"
+			case `[]int{0}`:
+				return `[0]`
+			case `[]string{""}`:
+				return `[""]`
+			case "[]Scope{ScopeNone}":
+				return `["(no scope)"]`
 			}
 			log.Fatalf("Unhandled zero value: %q", v)
 			return ""
@@ -144,7 +150,18 @@ func (t *templateData) processAST(f *ast.File) error {
 					logf("Got FuncDecl: Name=%q, id.Name=%#v", fn.Name.Name, id.Name)
 					t.StringFuncs[id.Name] = true
 				} else {
-					logf("Ignoring FuncDecl: Name=%q, Type=%T", fn.Name.Name, fn.Recv.List[0].Type)
+					star, ok := fn.Recv.List[0].Type.(*ast.StarExpr)
+					if ok && fn.Name.Name == "String" {
+						id, ok := star.X.(*ast.Ident)
+						if ok {
+							logf("Got FuncDecl: Name=%q, id.Name=%#v", fn.Name.Name, id.Name)
+							t.StringFuncs[id.Name] = true
+						} else {
+							logf("Ignoring FuncDecl: Name=%q, Type=%T", fn.Name.Name, fn.Recv.List[0].Type)
+						}
+					} else {
+						logf("Ignoring FuncDecl: Name=%q, Type=%T", fn.Name.Name, fn.Recv.List[0].Type)
+					}
 				}
 			} else {
 				logf("Ignoring FuncDecl: Name=%q, fn=%#v", fn.Name.Name, fn)
@@ -157,6 +174,7 @@ func (t *templateData) processAST(f *ast.File) error {
 			logf("Ignoring AST decl type %T", decl)
 			continue
 		}
+
 		for _, spec := range gd.Specs {
 			ts, ok := spec.(*ast.TypeSpec)
 			if !ok {
@@ -188,6 +206,13 @@ func (t *templateData) processAST(f *ast.File) error {
 					continue
 				}
 
+				if at, ok := field.Type.(*ast.ArrayType); ok {
+					if id, ok := at.Elt.(*ast.Ident); ok {
+						t.addIdentSlice(id, ts.Name.String(), fieldName.String())
+						continue
+					}
+				}
+
 				se, ok := field.Type.(*ast.StarExpr)
 				if !ok {
 					logf("Ignoring type %T for Name=%q, FieldName=%q", field.Type, ts.Name.String(), fieldName.String())
@@ -272,6 +297,32 @@ func (t *templateData) addIdentPtr(x *ast.Ident, receiverType, fieldName string)
 	t.StructFields[receiverType] = append(t.StructFields[receiverType], newStructField(receiverType, fieldName, x.String(), zeroValue, namedStruct))
 }
 
+func (t *templateData) addIdentSlice(x *ast.Ident, receiverType, fieldName string) {
+	var zeroValue string
+	var namedStruct = false
+	switch x.String() {
+	case "int":
+		zeroValue = "[]int{0}"
+	case "int64":
+		zeroValue = "[]int64{0}"
+	case "float64":
+		zeroValue = "[]float64{0}"
+	case "string":
+		zeroValue = `[]string{""}`
+	case "bool":
+		zeroValue = "[]bool{false}"
+	case "Scope":
+		zeroValue = "[]Scope{ScopeNone}"
+	// case "Timestamp":
+	// 	zeroValue = "&Timestamp{}"
+	default:
+		zeroValue = "nil"
+		namedStruct = true
+	}
+
+	t.StructFields[receiverType] = append(t.StructFields[receiverType], newStructField(receiverType, fieldName, x.String(), zeroValue, namedStruct))
+}
+
 func (t *templateData) dump() error {
 	if len(t.StructFields) == 0 {
 		logf("No StructFields for %v; skipping.", t.filename)

github/gists.go

@@ -279,6 +279,7 @@ func (s *GistsService) Delete(ctx context.Context, id string) (*Response, error)
 	if err != nil {
 		return nil, err
 	}
+
 	return s.client.Do(ctx, req, nil)
 }
 
@@ -291,6 +292,7 @@ func (s *GistsService) Star(ctx context.Context, id string) (*Response, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	return s.client.Do(ctx, req, nil)
 }
 
@@ -303,6 +305,7 @@ func (s *GistsService) Unstar(ctx context.Context, id string) (*Response, error)
 	if err != nil {
 		return nil, err
 	}
+
 	return s.client.Do(ctx, req, nil)
 }
 
@@ -315,9 +318,14 @@ func (s *GistsService) IsStarred(ctx context.Context, id string) (bool, *Respons
 	if err != nil {
 		return false, nil, err
 	}
+
 	resp, err := s.client.Do(ctx, req, nil)
 	starred, err := parseBoolResponse(err)
-	return starred, resp, err
+	if err != nil {
+		return false, resp, err
+	}
+
+	return starred, resp, nil
 }
 
 // Fork a gist.