Skip to content

feat: add reviewers parameter to create_pull_request #814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

yukukotani
Copy link

Summary

This PR adds support for specifying reviewers when creating a pull request, bringing feature parity with the update_pull_request tool which already supports this functionality.

Changes

  • ✨ Add reviewers parameter to create_pull_request tool
  • 🔧 Request reviewers immediately after PR creation using GitHub API
  • 🔄 Refresh PR data after adding reviewers to return complete information
  • ✅ Add comprehensive test coverage for the new parameter

Implementation Details

The implementation follows the same pattern as update_pull_request:

  1. Accept an array of GitHub usernames in the reviewers parameter
  2. After successfully creating the PR, call RequestReviewers API if reviewers are specified
  3. Fetch the updated PR data to include reviewer information in the response

Testing

  • Added test case for PR creation with reviewers
  • Updated tool snapshot to include the new parameter
  • All existing tests continue to pass
  • Linter checks pass with 0 issues

Related

This brings consistency with PR #285 which added reviewers parameter to update_pull_request.

🤖 Generated with Claude Code

- Add reviewers parameter to create_pull_request tool matching update_pull_request
- Request reviewers after PR creation using GitHub API
- Refresh PR data after adding reviewers to return complete information
- Add comprehensive test coverage for the new parameter

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@yukukotani yukukotani marked this pull request as ready for review August 4, 2025 00:55
@yukukotani yukukotani requested a review from a team as a code owner August 4, 2025 00:55
@Copilot Copilot AI review requested due to automatic review settings August 4, 2025 00:55
Copilot

This comment was marked as outdated.

yukukotani and others added 2 commits August 4, 2025 10:14
…equest

- Add reviewers parameter documentation to match the implementation
- Maintain alphabetical order of parameters in the list

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Change array[string] to string[] to match existing documentation style
- Consistent with other array parameters in the README

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@Copilot Copilot AI review requested due to automatic review settings August 4, 2025 01:16
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for specifying reviewers when creating a pull request, bringing feature parity with the existing update_pull_request functionality.

  • Add reviewers parameter to create_pull_request tool for requesting reviews upon PR creation
  • Implement reviewer request flow that calls GitHub's RequestReviewers API after PR creation
  • Add comprehensive test coverage for the new parameter functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
pkg/github/pullrequests.go Adds reviewers parameter and implements reviewer request logic after PR creation
pkg/github/pullrequests_test.go Adds test coverage for PR creation with reviewers functionality
pkg/github/toolsnaps/create_pull_request.snap Updates tool schema snapshot to include reviewers parameter
README.md Documents the new reviewers parameter in the tool documentation

if reviewResp.StatusCode != http.StatusCreated && reviewResp.StatusCode != http.StatusOK {
body, err := io.ReadAll(reviewResp.Body)
if err != nil {
return nil, fmt.Errorf("failed to read response body: %w", err)
Copy link
Preview

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return statement returns a nil *mcp.CallToolResult and an error, which is inconsistent with the function signature that expects (*mcp.CallToolResult, error). This should return an mcp.NewToolResultError instead of returning nil and an error.

Suggested change
return nil, fmt.Errorf("failed to read response body: %w", err)
return mcp.NewToolResultError(fmt.Sprintf("failed to read response body: %v", err)), nil

Copilot uses AI. Check for mistakes.

err,
), nil
}
defer func() { _ = resp.Body.Close() }()
Copy link
Preview

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The defer statement for closing resp.Body should include a nil check for resp to avoid potential panics. This should be: defer func() { if resp != nil && resp.Body != nil { _ = resp.Body.Close() } }()

Suggested change
defer func() { _ = resp.Body.Close() }()
defer func() {
if resp != nil && resp.Body != nil {
_ = resp.Body.Close()
}
}()

Copilot uses AI. Check for mistakes.

@Ali012101210121
Copy link

Ali012101210121 commented Aug 4, 2025 via email

@Ali012101210121
Copy link

Ali012101210121 commented Aug 4, 2025 via email

@yukukotani
Copy link
Author

wtf is this spam

@Ali012101210121
Copy link

Ali012101210121 commented Aug 4, 2025 via email

@Ali012101210121
Copy link

Ali012101210121 commented Aug 4, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants