refactor: address code style nitpicks in MarkPullRequestReadyForReview

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