feat: add reviewers parameter to UpdatePullRequest #285
Conversation
|
Hi @MayorFaj, can you please update the pr so we can get it merged? :) |
…ation- go run ./cmd/github-mcp-server generate-docs
|
@JoannaaKL the PR has been updated |
Could you update again because is out-of-date? Great PR! |
pkg/github/pullrequests.go
Outdated
| } | ||
|
|
||
| // If no updates and no reviewers, return error | ||
| if !updateNeeded && len(reviewers) == 0 { |
There was a problem hiding this comment.
Can we move this check before the line 316?
pkg/github/pullrequests.go
Outdated
| } else { | ||
| // If no update needed, just get the current PR | ||
| var ghResp *github.Response | ||
| pr, ghResp, err = client.PullRequests.Get(ctx, owner, repo, pullNumber) |
There was a problem hiding this comment.
We we need this else? If no updates are needed and no new reviewers were requested we will just return an error (please see comment below).
* initial impl of pull request draft state update * appease linter * update README * add nosec * fixed err return type for json marshalling * add gql test
…b#775) * make repo optional, and default to .github when not provided. improve tool description * autogen * update tests * small copy paste error fixes
…or reviewer updates
|
@JoannaaKL , if you get the chance could you please review this PR again so it can be merged asap. Has changes get merged to main, I am having to resolve conflicts, which is time consuming |
| if tc.expectedPR.Title != nil { | ||
| assert.Equal(t, *tc.expectedPR.Title, *returnedPR.Title) | ||
| } | ||
| if tc.expectedPR.Body != nil { | ||
| assert.Equal(t, *tc.expectedPR.Body, *returnedPR.Body) | ||
| } | ||
| if tc.expectedPR.State != nil { | ||
| assert.Equal(t, *tc.expectedPR.State, *returnedPR.State) | ||
| } | ||
| if tc.expectedPR.Base != nil && tc.expectedPR.Base.Ref != nil { | ||
| assert.NotNil(t, returnedPR.Base) | ||
| assert.Equal(t, *tc.expectedPR.Base.Ref, *returnedPR.Base.Ref) | ||
| } | ||
| if tc.expectedPR.MaintainerCanModify != nil { | ||
| assert.Equal(t, *tc.expectedPR.MaintainerCanModify, *returnedPR.MaintainerCanModify) | ||
| } |
There was a problem hiding this comment.
This change is out of scope for this PR
| mock.WithRequestMatch( | ||
| mock.GetReposPullsByOwnerByRepoByPullNumber, | ||
| mockNoUpdatePR, | ||
| ), |
There was a problem hiding this comment.
Getting the PR here is not needed
| mockNoUpdatePR := &github.PullRequest{ | ||
| Number: github.Ptr(42), | ||
| Title: github.Ptr("Test PR"), | ||
| State: github.Ptr("open"), | ||
| } |
There was a problem hiding this comment.
This can be removed as well due to the referencing test not being required
This pull request introduces the ability to request reviewers when updating a pull request. The changes include updates to the
UpdatePullRequestfunction to handle reviewers, modifications to the test cases to cover the new functionality, and updates to the documentation.Enhancements to
UpdatePullRequestfunction:UpdatePullRequestfunction. (pkg/github/pullrequests.go) [1] [2] [3]pkg/github/pullrequests.go)Documentation updates:
README.mdto include the newreviewersparameter in the documentation for updating pull requests. (README.md)Test case updates:
pkg/github/pullrequests_test.go) [1] [2] [3] [4] [5]Closes: #259