From b17b12cc0e052f958a400c65d6471c1be5deda63 Mon Sep 17 00:00:00 2001 From: RaviTeja Date: Sun, 6 Aug 2017 11:32:33 +0530 Subject: [PATCH 1/4] Add Preview Support for Team Review Requests --- github/github.go | 3 +++ github/pulls_reviewers.go | 38 +++++++++++++++++++++++++--------- github/pulls_reviewers_test.go | 31 ++++++++++++++++++--------- 3 files changed, 52 insertions(+), 20 deletions(-) diff --git a/github/github.go b/github/github.go index e86a0ebf00a..33fb707f72f 100644 --- a/github/github.go +++ b/github/github.go @@ -99,6 +99,9 @@ const ( // https://developer.github.com/changes/2017-07-17-update-topics-on-repositories/ mediaTypeTopicsPreview = "application/vnd.github.mercy-preview+json" + + // https://developer.github.com/changes/2017-07-26-team-review-request-thor-preview/ + mediaTypeTeamReviewPreview = "application/vnd.github.thor-preview+json" ) // A Client manages communication with the GitHub API. diff --git a/github/pulls_reviewers.go b/github/pulls_reviewers.go index bd94a6599b2..1b9678d143e 100644 --- a/github/pulls_reviewers.go +++ b/github/pulls_reviewers.go @@ -13,18 +13,22 @@ import ( // RequestReviewers creates a review request for the provided GitHub users for the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#create-a-review-request -func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo string, number int, logins []string) (*PullRequest, *Response, error) { +func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo string, number int, logins []string, teams []string) (*PullRequest, *Response, error) { u := fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number) reviewers := struct { - Reviewers []string `json:"reviewers,omitempty"` + Reviewers []string `json:"reviewers,omitempty"` + TeamReviewers []string `json:"team_reviewers,omitempty"` }{ - Reviewers: logins, + Reviewers: logins, + TeamReviewers: teams, } req, err := s.client.NewRequest("POST", u, &reviewers) if err != nil { return nil, nil, err } + // TODO: remove custom Accept header when this API fully launches. + req.Header.Set("Accept", mediaTypeTeamReviewPreview) r := new(PullRequest) resp, err := s.client.Do(ctx, req, r) @@ -35,10 +39,16 @@ func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo return r, resp, nil } +// Reviewers represents reviewers of a pull request. +type Reviewers struct { + Users []*User `json:"users,omitempty"` + Teams []*Team `json:"teams,omitempty"` +} + // ListReviewers lists users whose reviews have been requested on the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#list-review-requests -func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo string, number int, opt *ListOptions) ([]*User, *Response, error) { +func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo string, number int, opt *ListOptions) (*Reviewers, *Response, error) { u := fmt.Sprintf("repos/%v/%v/pulls/%d/requested_reviewers", owner, repo, number) u, err := addOptions(u, opt) if err != nil { @@ -50,30 +60,38 @@ func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo str return nil, nil, err } - var users []*User - resp, err := s.client.Do(ctx, req, &users) + // TODO: remove custom Accept header when this API fully launches. + req.Header.Set("Accept", mediaTypeTeamReviewPreview) + + reviewers := new(Reviewers) + resp, err := s.client.Do(ctx, req, reviewers) if err != nil { return nil, resp, err } - return users, resp, nil + return reviewers, resp, nil } // RemoveReviewers removes the review request for the provided GitHub users for the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#delete-a-review-request -func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, logins []string) (*Response, error) { +func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, logins []string, teams []string) (*Response, error) { u := fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number) reviewers := struct { - Reviewers []string `json:"reviewers,omitempty"` + Reviewers []string `json:"reviewers,omitempty"` + TeamReviewers []string `json:"team_reviewers,omitempty"` }{ - Reviewers: logins, + Reviewers: logins, + TeamReviewers: teams, } req, err := s.client.NewRequest("DELETE", u, &reviewers) if err != nil { return nil, err } + // TODO: remove custom Accept header when this API fully launches. + req.Header.Set("Accept", mediaTypeTeamReviewPreview) + return s.client.Do(ctx, req, reviewers) } diff --git a/github/pulls_reviewers_test.go b/github/pulls_reviewers_test.go index a60f2e77b38..bd0b2637207 100644 --- a/github/pulls_reviewers_test.go +++ b/github/pulls_reviewers_test.go @@ -19,12 +19,13 @@ func TestRequestReviewers(t *testing.T) { mux.HandleFunc("/repos/o/r/pulls/1/requested_reviewers", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "POST") - testBody(t, r, `{"reviewers":["octocat","googlebot"]}`+"\n") + testBody(t, r, `{"reviewers":["octocat","googlebot"],"team_reviewers":["justice-league","injustice-league"]}`+"\n") + testHeader(t, r, "Accept", mediaTypeTeamReviewPreview) fmt.Fprint(w, `{"number":1}`) }) // This returns a PR, unmarshalling of which is tested elsewhere - pull, _, err := client.PullRequests.RequestReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"}) + pull, _, err := client.PullRequests.RequestReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"}, []string{"justice-league", "injustice-league"}) if err != nil { t.Errorf("PullRequests.RequestReviewers returned error: %v", err) } @@ -40,10 +41,11 @@ func TestRemoveReviewers(t *testing.T) { mux.HandleFunc("/repos/o/r/pulls/1/requested_reviewers", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "DELETE") - testBody(t, r, `{"reviewers":["octocat","googlebot"]}`+"\n") + testHeader(t, r, "Accept", mediaTypeTeamReviewPreview) + testBody(t, r, `{"reviewers":["octocat","googlebot"],"team_reviewers":["justice-league"]}`+"\n") }) - _, err := client.PullRequests.RemoveReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"}) + _, err := client.PullRequests.RemoveReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"}, []string{"justice-league"}) if err != nil { t.Errorf("PullRequests.RemoveReviewers returned error: %v", err) } @@ -55,7 +57,8 @@ func TestListReviewers(t *testing.T) { mux.HandleFunc("/repos/o/r/pulls/1/requested_reviewers", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprint(w, `[{"login":"octocat","id":1}]`) + testHeader(t, r, "Accept", mediaTypeTeamReviewPreview) + fmt.Fprint(w, `{"users":[{"login":"octocat","id":1}],"teams":[{"id":1,"name":"Justice League"}]}`) }) reviewers, _, err := client.PullRequests.ListReviewers(context.Background(), "o", "r", 1, nil) @@ -63,10 +66,18 @@ func TestListReviewers(t *testing.T) { t.Errorf("PullRequests.ListReviewers returned error: %v", err) } - want := []*User{ - { - Login: String("octocat"), - ID: Int(1), + want := &Reviewers{ + Users: []*User{ + { + Login: String("octocat"), + ID: Int(1), + }, + }, + Teams: []*Team{ + { + ID: Int(1), + Name: String("Justice League"), + }, }, } if !reflect.DeepEqual(reviewers, want) { @@ -83,7 +94,7 @@ func TestListReviewers_withOptions(t *testing.T) { testFormValues(t, r, values{ "page": "2", }) - fmt.Fprint(w, `[]`) + fmt.Fprint(w, `{}`) }) _, _, err := client.PullRequests.ListReviewers(context.Background(), "o", "r", 1, &ListOptions{Page: 2}) From b5050395202b4e4b16680ef793d6e05896350e65 Mon Sep 17 00:00:00 2001 From: RaviTeja Date: Mon, 7 Aug 2017 10:25:26 +0530 Subject: [PATCH 2/4] Bump the libraryVersion as this change involves a breaking API and also, update the docs to reflect the changes --- github/github.go | 2 +- github/pulls_reviewers.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/github/github.go b/github/github.go index 33fb707f72f..004e8b86e03 100644 --- a/github/github.go +++ b/github/github.go @@ -27,7 +27,7 @@ import ( ) const ( - libraryVersion = "8" + libraryVersion = "9" defaultBaseURL = "https://api.github.com/" uploadBaseURL = "https://uploads.github.com/" userAgent = "go-github/" + libraryVersion diff --git a/github/pulls_reviewers.go b/github/pulls_reviewers.go index 1b9678d143e..f44a0722395 100644 --- a/github/pulls_reviewers.go +++ b/github/pulls_reviewers.go @@ -10,7 +10,7 @@ import ( "fmt" ) -// RequestReviewers creates a review request for the provided GitHub users for the specified pull request. +// RequestReviewers creates a review request for the provided GitHub users and teams for the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#create-a-review-request func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo string, number int, logins []string, teams []string) (*PullRequest, *Response, error) { @@ -45,7 +45,7 @@ type Reviewers struct { Teams []*Team `json:"teams,omitempty"` } -// ListReviewers lists users whose reviews have been requested on the specified pull request. +// ListReviewers lists users and teams whose reviews have been requested on the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#list-review-requests func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo string, number int, opt *ListOptions) (*Reviewers, *Response, error) { @@ -72,7 +72,7 @@ func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo str return reviewers, resp, nil } -// RemoveReviewers removes the review request for the provided GitHub users for the specified pull request. +// RemoveReviewers removes the review request for the provided GitHub users and teams for the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#delete-a-review-request func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, logins []string, teams []string) (*Response, error) { From de6c06bd8e9f5caf5da5b71af0c4636139509ce0 Mon Sep 17 00:00:00 2001 From: RaviTeja Date: Wed, 9 Aug 2017 23:20:57 +0530 Subject: [PATCH 3/4] Making the suggested API changes --- github/pulls_reviewers.go | 45 ++++++++++++++-------------------- github/pulls_reviewers_test.go | 4 +-- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/github/pulls_reviewers.go b/github/pulls_reviewers.go index f44a0722395..737c7315e4f 100644 --- a/github/pulls_reviewers.go +++ b/github/pulls_reviewers.go @@ -10,23 +10,28 @@ import ( "fmt" ) -// RequestReviewers creates a review request for the provided GitHub users and teams for the specified pull request. +// Reviewers represents reviewers of a pull request. +type Reviewers struct { + Users []*User `json:"users,omitempty"` + Teams []*Team `json:"teams,omitempty"` +} + +// ReviewersRequest specifies users and teams for a pull request review request. +type ReviewersRequest struct { + Reviewers []string `json:"reviewers,omitempty"` + TeamReviewers []string `json:"team_reviewers,omitempty"` +} + +// RequestReviewers creates a review request for the provided reviewers for the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#create-a-review-request -func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo string, number int, logins []string, teams []string) (*PullRequest, *Response, error) { +func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo string, number int, reviewers ReviewersRequest) (*PullRequest, *Response, error) { u := fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number) - - reviewers := struct { - Reviewers []string `json:"reviewers,omitempty"` - TeamReviewers []string `json:"team_reviewers,omitempty"` - }{ - Reviewers: logins, - TeamReviewers: teams, - } req, err := s.client.NewRequest("POST", u, &reviewers) if err != nil { return nil, nil, err } + // TODO: remove custom Accept header when this API fully launches. req.Header.Set("Accept", mediaTypeTeamReviewPreview) @@ -39,13 +44,7 @@ func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo return r, resp, nil } -// Reviewers represents reviewers of a pull request. -type Reviewers struct { - Users []*User `json:"users,omitempty"` - Teams []*Team `json:"teams,omitempty"` -} - -// ListReviewers lists users and teams whose reviews have been requested on the specified pull request. +// ListReviewers lists reviewers whose reviews have been requested on the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#list-review-requests func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo string, number int, opt *ListOptions) (*Reviewers, *Response, error) { @@ -72,19 +71,11 @@ func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo str return reviewers, resp, nil } -// RemoveReviewers removes the review request for the provided GitHub users and teams for the specified pull request. +// RemoveReviewers removes the review request for the provided reviewers for the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#delete-a-review-request -func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, logins []string, teams []string) (*Response, error) { +func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, reviewers ReviewersRequest) (*Response, error) { u := fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number) - - reviewers := struct { - Reviewers []string `json:"reviewers,omitempty"` - TeamReviewers []string `json:"team_reviewers,omitempty"` - }{ - Reviewers: logins, - TeamReviewers: teams, - } req, err := s.client.NewRequest("DELETE", u, &reviewers) if err != nil { return nil, err diff --git a/github/pulls_reviewers_test.go b/github/pulls_reviewers_test.go index bd0b2637207..fddea6d0891 100644 --- a/github/pulls_reviewers_test.go +++ b/github/pulls_reviewers_test.go @@ -25,7 +25,7 @@ func TestRequestReviewers(t *testing.T) { }) // This returns a PR, unmarshalling of which is tested elsewhere - pull, _, err := client.PullRequests.RequestReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"}, []string{"justice-league", "injustice-league"}) + pull, _, err := client.PullRequests.RequestReviewers(context.Background(), "o", "r", 1, ReviewersRequest{Reviewers: []string{"octocat", "googlebot"}, TeamReviewers: []string{"justice-league", "injustice-league"}}) if err != nil { t.Errorf("PullRequests.RequestReviewers returned error: %v", err) } @@ -45,7 +45,7 @@ func TestRemoveReviewers(t *testing.T) { testBody(t, r, `{"reviewers":["octocat","googlebot"],"team_reviewers":["justice-league"]}`+"\n") }) - _, err := client.PullRequests.RemoveReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"}, []string{"justice-league"}) + _, err := client.PullRequests.RemoveReviewers(context.Background(), "o", "r", 1, ReviewersRequest{Reviewers: []string{"octocat", "googlebot"}, TeamReviewers: []string{"justice-league"}}) if err != nil { t.Errorf("PullRequests.RemoveReviewers returned error: %v", err) } From cb6757206191ecbe6caec407e14e3e22e0ae6af3 Mon Sep 17 00:00:00 2001 From: RaviTezu Date: Thu, 10 Aug 2017 01:07:32 +0530 Subject: [PATCH 4/4] Reordering the structs; as RevieweresRequest struct is used for input and Revewiers struct is used for output --- github/pulls_reviewers.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/github/pulls_reviewers.go b/github/pulls_reviewers.go index 737c7315e4f..15b0d8464e6 100644 --- a/github/pulls_reviewers.go +++ b/github/pulls_reviewers.go @@ -10,18 +10,18 @@ import ( "fmt" ) -// Reviewers represents reviewers of a pull request. -type Reviewers struct { - Users []*User `json:"users,omitempty"` - Teams []*Team `json:"teams,omitempty"` -} - // ReviewersRequest specifies users and teams for a pull request review request. type ReviewersRequest struct { Reviewers []string `json:"reviewers,omitempty"` TeamReviewers []string `json:"team_reviewers,omitempty"` } +// Reviewers represents reviewers of a pull request. +type Reviewers struct { + Users []*User `json:"users,omitempty"` + Teams []*Team `json:"teams,omitempty"` +} + // RequestReviewers creates a review request for the provided reviewers for the specified pull request. // // GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#create-a-review-request