Fix bugs, improve code quality, upgrade go-github to v74

approval.go

@@ -4,9 +4,10 @@ import (
 	"context"
 	"fmt"
 	"regexp"
+	"slices"
 	"strings"
 
-	"github.com/google/go-github/v61/github"
+	"github.com/google/go-github/v74/github"
 )
 
 type approvalEnvironment struct {
@@ -55,7 +56,7 @@ func (a *approvalEnvironment) createApprovalIssue(ctx context.Context) error {
 	issueApproversText := "Anyone can approve."
 	assignees := []string{a.workflowInitiator}
 	if len(a.issueApprovers) > 0 {
-		issueApproversText = fmt.Sprintf("%s", a.issueApprovers)
+		issueApproversText = strings.Join(a.issueApprovers, ", ")
 		assignees = a.issueApprovers
 	}
 
@@ -112,18 +113,18 @@ func approvalFromComments(comments []*github.IssueComment, approvers []string, m
 	for _, comment := range comments {
 		commentUser := comment.User.GetLogin()
 
-		if approversIndex(disallowedUsers, commentUser) >= 0 {
+		if slices.Contains(disallowedUsers, commentUser) {
 			continue
 		}
-		if approversIndex(approvals, commentUser) >= 0 {
+		if slices.Contains(approvals, commentUser) {
 			continue
 		}
-		if len(approvers) > 0 && approversIndex(approvers, commentUser) < 0 {
+		if len(approvers) > 0 && !slices.Contains(approvers, commentUser) {
 			continue
 		}
 
 		commentBody := comment.GetBody()
-		isApprovalComment, err := isApproved(commentBody)
+		isApprovalComment, err := matchesWordList(commentBody, approvedWords)
 		if err != nil {
 			return approvalStatusPending, err
 		}
@@ -135,7 +136,7 @@ func approvalFromComments(comments []*github.IssueComment, approvers []string, m
 			continue
 		}
 
-		isDenialComment, err := isDenied(commentBody)
+		isDenialComment, err := matchesWordList(commentBody, deniedWords)
 		if err != nil {
 			return approvalStatusPending, err
 		}
@@ -147,46 +148,16 @@ func approvalFromComments(comments []*github.IssueComment, approvers []string, m
 	return approvalStatusPending, nil
 }
 
-func approversIndex(approvers []string, name string) int {
-	for idx, approver := range approvers {
-		if approver == name {
-			return idx
-		}
-	}
-	return -1
-}
-
-func isApproved(commentBody string) (bool, error) {
-	for _, approvedWord := range approvedWords {
-		re, err := regexp.Compile(fmt.Sprintf("(?i)^%s[.!]*\n*\\s*$", regexp.QuoteMeta(approvedWord)))
-		if err != nil {
-			fmt.Printf("Error parsing. %v", err)
-			return false, err
-		}
-
-		matched := re.MatchString(commentBody)
-
-		if matched {
-			return true, nil
-		}
-	}
-
-	return false, nil
-}
-
-func isDenied(commentBody string) (bool, error) {
-	for _, deniedWord := range deniedWords {
-		re, err := regexp.Compile(fmt.Sprintf("(?i)^%s[.!]*\n*\\s*$", regexp.QuoteMeta(deniedWord)))
+func matchesWordList(commentBody string, words []string) (bool, error) {
+	for _, word := range words {
+		re, err := regexp.Compile(fmt.Sprintf("(?i)^%s[.!]*\n*\\s*$", regexp.QuoteMeta(word)))
 		if err != nil {
-			fmt.Printf("Error parsing. %v", err)
-			return false, err
+			return false, fmt.Errorf("error compiling pattern for %q: %w", word, err)
 		}
-		matched := re.MatchString(commentBody)
-		if matched {
+		if re.MatchString(commentBody) {
 			return true, nil
 		}
 	}
-
 	return false, nil
 }
 

approval_test.go

@@ -3,7 +3,7 @@ package main
 import (
 	"testing"
 
-	"github.com/google/go-github/v61/github"
+	"github.com/google/go-github/v74/github"
 )
 
 func TestApprovalFromComments(t *testing.T) {
@@ -337,7 +337,7 @@ func TestApprovedCommentBody(t *testing.T) {
 			}
 
 			// test
-			actual, err := isApproved(testCase.commentBody)
+			actual, err := matchesWordList(testCase.commentBody, approvedWords)
 			if err != nil {
 				t.Fatalf("error getting approval: %v", err)
 			}
@@ -473,9 +473,9 @@ func TestDeniedCommentBody(t *testing.T) {
 			}
 
 			// test
-			actual, err := isDenied(testCase.commentBody)
+			actual, err := matchesWordList(testCase.commentBody, deniedWords)
 			if err != nil {
-				t.Fatalf("error getting approval: %v", err)
+				t.Fatalf("error getting denial: %v", err)
 			}
 			if actual != testCase.isSuccess {
 				t.Fatalf("expected %v but got %v", testCase.isSuccess, actual)

approvers.go

@@ -7,44 +7,42 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/google/go-github/v61/github"
+	"github.com/google/go-github/v74/github"
 )
 
-func retrieveApprovers(client *github.Client, repoOwner string) ([]string, []string, error) {
+func retrieveApprovers(client *github.Client, repoOwner string) ([]string, []string, int, error) {
 	workflowInitiator := os.Getenv(envVarWorkflowInitiator)
 	shouldExcludeWorkflowInitiatorRaw := os.Getenv(envVarExcludeWorkflowInitiatorAsApprover)
 	shouldExcludeWorkflowInitiator, parseBoolErr := strconv.ParseBool(shouldExcludeWorkflowInitiatorRaw)
 	if parseBoolErr != nil {
-		return nil, nil, fmt.Errorf("error parsing exclude-workflow-initiator-as-approver flag: %w", parseBoolErr)
+		return nil, nil, 0, fmt.Errorf("error parsing exclude-workflow-initiator-as-approver flag: %w", parseBoolErr)
 	}
 
-	approvers := []string{}
+	var approvers []string
 	requiredApproversRaw := os.Getenv(envVarApprovers)
-	requiredApprovers := []string{}
+	var requiredApprovers []string
 	if requiredApproversRaw != "" {
 		requiredApprovers = strings.Split(requiredApproversRaw, ",")
 	}
 
-	minimumApprovalsRaw := os.Getenv(envVarMinimumApprovals)
-	minimumApprovals := len(approvers)
-
 	var disallowedUsers []string
 	if shouldExcludeWorkflowInitiator {
 		disallowedUsers = []string{workflowInitiator}
-	} else {
-		disallowedUsers = []string{}
 	}
 
 	if len(requiredApprovers) == 0 {
-		return []string{}, disallowedUsers, nil
+		return nil, disallowedUsers, 0, nil
 	}
 
 	for i := range requiredApprovers {
 		requiredApprovers[i] = strings.TrimSpace(requiredApprovers[i])
 	}
 
 	for _, approverUser := range requiredApprovers {
-		expandedUsers := expandGroupFromUser(client, repoOwner, approverUser, workflowInitiator, shouldExcludeWorkflowInitiator)
+		expandedUsers, err := expandGroupFromUser(client, repoOwner, approverUser, workflowInitiator, shouldExcludeWorkflowInitiator)
+		if err != nil {
+			return nil, nil, 0, fmt.Errorf("error resolving approver %q: %w", approverUser, err)
+		}
 		if expandedUsers != nil {
 			approvers = append(approvers, expandedUsers...)
 		} else if strings.EqualFold(workflowInitiator, approverUser) && shouldExcludeWorkflowInitiator {
@@ -56,33 +54,36 @@ func retrieveApprovers(client *github.Client, repoOwner string) ([]string, []str
 
 	approvers = deduplicateUsers(approvers)
 
-	var err error
-	if minimumApprovalsRaw != "" {
-		minimumApprovals, err = strconv.Atoi(minimumApprovalsRaw)
+	minimumApprovals := len(approvers)
+	if raw := os.Getenv(envVarMinimumApprovals); raw != "" {
+		var err error
+		minimumApprovals, err = strconv.Atoi(raw)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error parsing minimum number of approvals: %w", err)
+			return nil, nil, 0, fmt.Errorf("error parsing minimum number of approvals: %w", err)
 		}
 	}
 
 	if minimumApprovals > len(approvers) {
-		return nil, nil, fmt.Errorf("error: minimum required approvals (%d) is greater than the total number of approvers (%d)", minimumApprovals, len(approvers))
+		return nil, nil, 0, fmt.Errorf("error: minimum required approvals (%d) is greater than the total number of approvers (%d)", minimumApprovals, len(approvers))
 	}
 
-	return approvers, disallowedUsers, nil
+	return approvers, disallowedUsers, minimumApprovals, nil
 }
 
-func expandGroupFromUser(client *github.Client, org, userOrTeam string, workflowInitiator string, shouldExcludeWorkflowInitiator bool) []string {
+func expandGroupFromUser(client *github.Client, org, userOrTeam string, workflowInitiator string, shouldExcludeWorkflowInitiator bool) ([]string, error) {
 	fmt.Printf("Attempting to expand user %s/%s as a group (may not succeed)\n", org, userOrTeam)
 
 	// GitHub replaces periods in the team name with hyphens. If a period is
 	// passed to the request it would result in a 404. So we need to replace
-	// and occurrences with a hyphen.
+	// any occurrences with a hyphen.
 	formattedUserOrTeam := strings.ReplaceAll(userOrTeam, ".", "-")
 
-	users, _, err := client.Teams.ListTeamMembersBySlug(context.Background(), org, formattedUserOrTeam, &github.TeamListTeamMembersOptions{})
+	users, resp, err := client.Teams.ListTeamMembersBySlug(context.Background(), org, formattedUserOrTeam, &github.TeamListTeamMembersOptions{})
 	if err != nil {
-		fmt.Printf("%v\n", err)
-		return nil
+		if resp != nil && resp.StatusCode == 404 {
+			return nil, nil
+		}
+		return nil, fmt.Errorf("error expanding team %s/%s: %w", org, userOrTeam, err)
 	}
 
 	userNames := make([]string, 0, len(users))
@@ -95,7 +96,7 @@ func expandGroupFromUser(client *github.Client, org, userOrTeam string, workflow
 		}
 	}
 
-	return userNames
+	return userNames, nil
 }
 
 func deduplicateUsers(users []string) []string {

constants.go

@@ -22,7 +22,7 @@ const (
 	envVarAdditionalApprovedWords            string = "INPUT_ADDITIONAL-APPROVED-WORDS"
 	envVarAdditionalDeniedWords              string = "INPUT_ADDITIONAL-DENIED-WORDS"
 
-	githubBaseURL string = "https://github.com"
+	githubBaseURL string = "https://github.com/"
 )
 
 var (

go.mod

@@ -5,7 +5,7 @@ go 1.24.0
 toolchain go1.24.7
 
 require (
-	github.com/google/go-github/v61 v61.0.0
+	github.com/google/go-github/v74 v74.0.0
 	golang.org/x/oauth2 v0.31.0
 )
 

go.sum

@@ -1,8 +1,8 @@
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
-github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
-github.com/google/go-github/v61 v61.0.0 h1:VwQCBwhyE9JclCI+22/7mLB1PuU9eowCXKY5pNlu1go=
-github.com/google/go-github/v61 v61.0.0/go.mod h1:0WR+KmsWX75G2EbpyGsGmradjo3IiciuI4BmdVCobQY=
+github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
+github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
+github.com/google/go-github/v74 v74.0.0 h1:yZcddTUn8DPbj11GxnMrNiAnXH14gNs559AsUpNpPgM=
+github.com/google/go-github/v74 v74.0.0/go.mod h1:ubn/YdyftV80VPSI26nSJvaEsTOnsjrxG3o9kJhcyak=
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
 github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
 golang.org/x/oauth2 v0.31.0 h1:8Fq0yVZLh4j4YA47vHKFTa9Ew5XIrCP8LC6UeNZnLxo=

main.go

@@ -8,7 +8,7 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/google/go-github/v61/github"
+	"github.com/google/go-github/v74/github"
 	"golang.org/x/oauth2"
 )
 
@@ -39,13 +39,15 @@ func newCommentLoopChannel(ctx context.Context, apprv *approvalEnvironment, clie
 				fmt.Printf("error getting comments: %v\n", err)
 				channel <- 1
 				close(channel)
+				return
 			}
 
 			approved, err := approvalFromComments(comments, apprv.issueApprovers, apprv.minimumApprovals, apprv.disallowedUsers)
 			if err != nil {
 				fmt.Printf("error getting approval from comments: %v\n", err)
 				channel <- 1
 				close(channel)
+				return
 			}
 			fmt.Printf("Workflow status: %s\n", approved)
 			switch approved {
@@ -59,16 +61,19 @@ func newCommentLoopChannel(ctx context.Context, apprv *approvalEnvironment, clie
 					fmt.Printf("error commenting on issue: %v\n", err)
 					channel <- 1
 					close(channel)
+					return
 				}
 				_, _, err = client.Issues.Edit(ctx, apprv.repoOwner, apprv.repo, apprv.approvalIssueNumber, &github.IssueRequest{State: &newState})
 				if err != nil {
 					fmt.Printf("error closing issue: %v\n", err)
 					channel <- 1
 					close(channel)
+					return
 				}
 				channel <- 0
 				fmt.Println("Workflow manual approval completed")
 				close(channel)
+				return
 			case approvalStatusDenied:
 				newState := "closed"
 				closeComment := "Request denied. Closing issue and failing workflow."
@@ -79,15 +84,18 @@ func newCommentLoopChannel(ctx context.Context, apprv *approvalEnvironment, clie
 					fmt.Printf("error commenting on issue: %v\n", err)
 					channel <- 1
 					close(channel)
+					return
 				}
 				_, _, err = client.Issues.Edit(ctx, apprv.repoOwner, apprv.repo, apprv.approvalIssueNumber, &github.IssueRequest{State: &newState})
 				if err != nil {
 					fmt.Printf("error closing issue: %v\n", err)
 					channel <- 1
 					close(channel)
+					return
 				}
 				channel <- 1
 				close(channel)
+				return
 			}
 
 			time.Sleep(pollingInterval)
@@ -160,23 +168,14 @@ func main() {
 		os.Exit(1)
 	}
 
-	approvers, disallowedUsers, err := retrieveApprovers(client, repoOwner)
+	approvers, disallowedUsers, minimumApprovals, err := retrieveApprovers(client, repoOwner)
 	if err != nil {
 		fmt.Printf("error retrieving approvers: %v\n", err)
 		os.Exit(1)
 	}
 
 	issueTitle := os.Getenv(envVarIssueTitle)
 	issueBody := os.Getenv(envVarIssueBody)
-	minimumApprovalsRaw := os.Getenv(envVarMinimumApprovals)
-	minimumApprovals := 0
-	if minimumApprovalsRaw != "" {
-		minimumApprovals, err = strconv.Atoi(minimumApprovalsRaw)
-		if err != nil {
-			fmt.Printf("error parsing minimum approvals: %v\n", err)
-			os.Exit(1)
-		}
-	}
 	workflowInitiator := os.Getenv(envVarWorkflowInitiator)
 	apprv, err := newApprovalEnvironment(client, repoFullName, repoOwner, runID, approvers, minimumApprovals, issueTitle, issueBody, disallowedUsers, workflowInitiator)
 	if err != nil {