refactor: address code style nitpicks in MarkPullRequestReadyForReview

efouts · efouts · commit c9cda908d31c · 2025-06-03T08:57:56.000-04:00
Based on code review feedback, made the following improvements:
- Rename 'variables' to 'vars' for consistency with other GraphQL functions
- Improve comment precision for GraphQL schema requirements

These changes enhance code consistency and documentation clarity while
maintaining full functionality. All tests, linting, and build checks pass.
diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go
@@ -1643,7 +1643,7 @@ func MarkPullRequestReadyForReview(getGQLClient GetGQLClientFn, t translations.T
 			// Get the GraphQL client
 			client, err := getGQLClient(ctx)
 			if err != nil {
-				return nil, fmt.Errorf("failed to get GitHub GraphQL client: %w", err)
+				return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GraphQL client: %v", err)), nil
 			}
 
 			// First, we need to get the GraphQL ID of the pull request
@@ -1656,13 +1656,13 @@ func MarkPullRequestReadyForReview(getGQLClient GetGQLClientFn, t translations.T
 				} `graphql:"repository(owner: $owner, name: $repo)"`
 			}
 
-			variables := map[string]any{
+			vars := map[string]any{
 				"owner": githubv4.String(params.Owner),
 				"repo":  githubv4.String(params.Repo),
 				"prNum": githubv4.Int(params.PullNumber),
 			}
 
-			if err := client.Query(ctx, &getPullRequestQuery, variables); err != nil {
+			if err := client.Query(ctx, &getPullRequestQuery, vars); err != nil {
 				return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %v", err)), nil
 			}
 
@@ -1675,7 +1675,7 @@ func MarkPullRequestReadyForReview(getGQLClient GetGQLClientFn, t translations.T
 			var markReadyForReviewMutation struct {
 				MarkPullRequestReadyForReview struct {
 					PullRequest struct {
-						ID githubv4.ID // We don't need this, but a selector is required or GQL complains
+						ID githubv4.ID // Required by GraphQL schema, but not used in response
 					}
 				} `graphql:"markPullRequestReadyForReview(input: $input)"`
 			}

Original file line number	Diff line number	Diff line change
`@@ -1643,7 +1643,7 @@ func MarkPullRequestReadyForReview(getGQLClient GetGQLClientFn, t translations.T`
`1643`	`1643`	`// Get the GraphQL client`
`1644`	`1644`	`client, err := getGQLClient(ctx)`
`1645`	`1645`	`if err != nil {`
`1646`		`- return nil, fmt.Errorf("failed to get GitHub GraphQL client: %w", err)`
	`1646`	`+ return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GraphQL client: %v", err)), nil`
`1647`	`1647`	`}`
`1648`	`1648`
`1649`	`1649`	`// First, we need to get the GraphQL ID of the pull request`
`@@ -1656,13 +1656,13 @@ func MarkPullRequestReadyForReview(getGQLClient GetGQLClientFn, t translations.T`
`1656`	`1656`	} `graphql:"repository(owner: $owner, name: $repo)"`
`1657`	`1657`	`}`
`1658`	`1658`
`1659`		`- variables := map[string]any{`
	`1659`	`+ vars := map[string]any{`
`1660`	`1660`	`"owner": githubv4.String(params.Owner),`
`1661`	`1661`	`"repo": githubv4.String(params.Repo),`
`1662`	`1662`	`"prNum": githubv4.Int(params.PullNumber),`
`1663`	`1663`	`}`
`1664`	`1664`
`1665`		`- if err := client.Query(ctx, &getPullRequestQuery, variables); err != nil {`
	`1665`	`+ if err := client.Query(ctx, &getPullRequestQuery, vars); err != nil {`
`1666`	`1666`	`return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %v", err)), nil`
`1667`	`1667`	`}`
`1668`	`1668`
`@@ -1675,7 +1675,7 @@ func MarkPullRequestReadyForReview(getGQLClient GetGQLClientFn, t translations.T`
`1675`	`1675`	`var markReadyForReviewMutation struct {`
`1676`	`1676`	`MarkPullRequestReadyForReview struct {`
`1677`	`1677`	`PullRequest struct {`
`1678`		`- ID githubv4.ID // We don't need this, but a selector is required or GQL complains`
	`1678`	`+ ID githubv4.ID // Required by GraphQL schema, but not used in response`
`1679`	`1679`	`}`
`1680`	`1680`	} `graphql:"markPullRequestReadyForReview(input: $input)"`
`1681`	`1681`	`}`