Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
354caf4
Add 'list all reviews' endpoint from new pull request review API
sahildua2305 Dec 16, 2016
36c6f8c
Add GetReview and ListReviewComments methods to PullRequest
sahildua2305 Dec 17, 2016
a8392c4
Update GetReview and ListReviewComments to include repo param
sahildua2305 Dec 17, 2016
1ce3518
Add CreateReview method to PullRequest service
sahildua2305 Dec 18, 2016
5182678
Modify CreateReview to add PullRequestReviewRequest struct
sahildua2305 Dec 18, 2016
178c4ed
Add SubmitReview method to PullRequest service
sahildua2305 Dec 18, 2016
a8df51b
Fix minor typos related to SubmitReview method
sahildua2305 Dec 18, 2016
cd49c5c
Add DismissReview method to PullRequest service
sahildua2305 Dec 18, 2016
167e217
Fix minor formatting issue with gofmt
sahildua2305 Dec 18, 2016
99df209
Create new struct PullRequestReveiwDismissalRequest
sahildua2305 Dec 21, 2016
c60f5b3
Rename 'id' param to 'reviewID' to make it more verbose
sahildua2305 Dec 21, 2016
cd5b665
Fix minor issue with comment for state
sahildua2305 Dec 21, 2016
109ded2
Add String methods and their tests for all new types
sahildua2305 Dec 23, 2016
2bd08d4
Add unit tests for invalid cases
sahildua2305 Dec 23, 2016
87729c2
Complete tests for invalid cases for all 6 new endpoints
sahildua2305 Dec 29, 2016
a176b03
Replace PullRequestReviewComment with PullRequestComment
sahildua2305 Dec 29, 2016
e3b5e69
Remove misleading comment from PullRequestReview
sahildua2305 Jan 5, 2017
a1bf055
Add a note and todo about known issue with SubmitReview
sahildua2305 Jan 6, 2017
91cc8ae
Add DeletePendingReview endpoint
sahildua2305 Jan 31, 2017
bfda8ba
Make minor changes as suggested by gmlewis
sahildua2305 Feb 1, 2017
36dbd69
Doc: Add comment about known error format issue
sahildua2305 Feb 2, 2017
0c0faa4
Fix typo and mention issue link as comment
sahildua2305 Feb 2, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ const (

// https://developer.github.com/changes/2017-01-05-commit-search-api/
mediaTypeCommitSearchPreview = "application/vnd.github.cloak-preview+json"

// https://developer.github.com/changes/2016-12-14-reviews-api/
mediaTypePullRequestReviewsPreview = "application/vnd.github.black-cat-preview+json"
)

// A Client manages communication with the GitHub API.
Expand Down
242 changes: 235 additions & 7 deletions github/pulls_reviews.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,243 @@

package github

import "time"
import (
"fmt"
"time"
)

// PullRequestReview represents a review of a pull request.
type PullRequestReview struct {
Copy link
Copy Markdown
Contributor

@haya14busa haya14busa Dec 17, 2016

Choose a reason for hiding this comment

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

https://developer.github.com/v3/pulls/reviews/#list-reviews-on-a-pull-request

Some fields of list review API response are missing.

  • commit_id
  • html_url
  • pull_request_url

You can see similar fields in PullRequestComment

type PullRequestComment struct {

And "state" seems UPPRECASE and can be "APPROVED", "DISMISSED", "COMMENTED".

PullRequestReview is also used in PullRequestReviewEvent https://developer.github.com/v3/activity/events/types/#pullrequestreviewevent , so maybe we should check consistencies and give feed backs to GitHub support perhaps if the differences are not reasonable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"state" also can be "CHANGES_REQUESTED"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I mean, "state" can be "ACCEPTED", "DISMISSED", "CHANGES_REQUESTED" or "COMMENTED" for Rest API we are working with now, but "state" also can be "approved", "rejected", or "commented" for pull-request event API. https://developer.github.com/v3/activity/events/types/#pullrequestreviewevent

PullRequestReview is used here as a field of event API.

Review *PullRequestReview `json:"review,omitempty"`

So, it's not a good idea to remove existed comment, i guess.
Or, we can remove this comment completely and leave users to check GitHub API document, but i'm not sure.

And, there is a typo. s/ACCEPTED/APPROVED/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I will prefer removing this comment totally. @haya14busa

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agree. I think it's ok to remove it.

ID *int `json:"id,omitempty"`
User *User `json:"user,omitempty"`
Body *string `json:"body,omitempty"`
SubmittedAt *time.Time `json:"submitted_at,omitempty"`
ID *int `json:"id,omitempty"`
User *User `json:"user,omitempty"`
Body *string `json:"body,omitempty"`
SubmittedAt *time.Time `json:"submitted_at,omitempty"`
CommitID *string `json:"commit_id,omitempty"`
HTMLURL *string `json:"html_url,omitempty"`
PullRequestURL *string `json:"pull_request_url,omitempty"`
State *string `json:"state,omitempty"`
}

func (p PullRequestReview) String() string {
return Stringify(p)
}

// DraftReviewComment represents a comment part of the review.
type DraftReviewComment struct {
Path *string `json:"path,omitempty"`
Position *int `json:"position,omitempty"`
Body *string `json:"body,omitempty"`
}

func (c DraftReviewComment) String() string {
return Stringify(c)
}

// PullRequestReviewRequest represents a request to create a review.
type PullRequestReviewRequest struct {
Body *string `json:"body,omitempty"`
Event *string `json:"event,omitempty"`
Comments []*DraftReviewComment `json:"comments,omitempty"`
}

func (r PullRequestReviewRequest) String() string {
return Stringify(r)
}

// PullRequestReviewDismissalRequest represents a request to dismiss a review.
type PullRequestReviewDismissalRequest struct {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I wish we could reduce the stutter on this type... it reminds me too much of Java. 😃

Copy link
Copy Markdown
Member Author

@sahildua2305 sahildua2305 Feb 1, 2017

Choose a reason for hiding this comment

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

One thing I can think of is to use PRReviewDismissalRequest but that will disturb the consistency. 😞

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agreed. I couldn't think of a better alternative. I was just lamenting. 😄

Message *string `json:"message,omitempty"`
}

func (r PullRequestReviewDismissalRequest) String() string {
return Stringify(r)
}

// ListReviews lists all reviews on the specified pull request.
//
// TODO: Follow up with GitHub support about an issue with this method's
// returned error format and remove this comment once it's fixed.
// Read more about it here - https://github.com/google/go-github/issues/540
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#list-reviews-on-a-pull-request
func (s *PullRequestsService) ListReviews(owner, repo string, number int) ([]*PullRequestReview, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", owner, repo, number)

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches
req.Header.Set("Accept", mediaTypePullRequestReviewsPreview)

var reviews []*PullRequestReview
resp, err := s.client.Do(req, &reviews)
if err != nil {
return nil, resp, err
}

return reviews, resp, nil
}

// GetReview fetches the specified pull request review.
//
// TODO: Follow up with GitHub support about an issue with this method's
// returned error format and remove this comment once it's fixed.
// Read more about it here - https://github.com/google/go-github/issues/540
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#get-a-single-review
func (s *PullRequestsService) GetReview(owner, repo string, number, reviewID int) (*PullRequestReview, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews/%d", owner, repo, number, reviewID)

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches
req.Header.Set("Accept", mediaTypePullRequestReviewsPreview)

review := new(PullRequestReview)
resp, err := s.client.Do(req, review)
if err != nil {
return nil, resp, err
}

return review, resp, nil
}

// DeletePendingReview deletes the specified pull request pending review.
//
// TODO: Follow up with GitHub support about an issue with this method's
// returned error format and remove this comment once it's fixed.
// Read more about it here - https://github.com/google/go-github/issues/540
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#delete-a-pending-review
func (s *PullRequestsService) DeletePendingReview(owner, repo string, number, reviewID int) (*PullRequestReview, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews/%d", owner, repo, number, reviewID)

req, err := s.client.NewRequest("DELETE", u, nil)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches
req.Header.Set("Accept", mediaTypePullRequestReviewsPreview)

review := new(PullRequestReview)
resp, err := s.client.Do(req, review)
if err != nil {
return nil, resp, err
}

return review, resp, nil
}

// ListReviewComments lists all the comments for the specified review.
//
// TODO: Follow up with GitHub support about an issue with this method's
// returned error format and remove this comment once it's fixed.
// Read more about it here - https://github.com/google/go-github/issues/540
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#get-a-single-reviews-comments
func (s *PullRequestsService) ListReviewComments(owner, repo string, number, reviewID int) ([]*PullRequestComment, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews/%d/comments", owner, repo, number, reviewID)

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches
req.Header.Set("Accept", mediaTypePullRequestReviewsPreview)

var comments []*PullRequestComment
resp, err := s.client.Do(req, &comments)
if err != nil {
return nil, resp, err
}

return comments, resp, nil
}

// CreateReview creates a new review on the specified pull request.
//
// TODO: Follow up with GitHub support about an issue with this method's
// returned error format and remove this comment once it's fixed.
// Read more about it here - https://github.com/google/go-github/issues/540
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#create-a-pull-request-review
func (s *PullRequestsService) CreateReview(owner, repo string, number int, review *PullRequestReviewRequest) (*PullRequestReview, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews", owner, repo, number)

req, err := s.client.NewRequest("POST", u, review)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches
req.Header.Set("Accept", mediaTypePullRequestReviewsPreview)

r := new(PullRequestReview)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's keep variable name consistent. In GetReview, The variable name is "review".
In Go, short name is preferred if it makes senses, so I think it's ok to use r, but in this case, i like "review".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, "review" is used as argument. Please ignore this.

resp, err := s.client.Do(req, r)
if err != nil {
return nil, resp, err
}

return r, resp, nil
}

// SubmitReview submits a specified review on the specified pull request.
//
// TODO: Follow up with GitHub support about an issue with this method's
// returned error format and remove this comment once it's fixed.
// Read more about it here - https://github.com/google/go-github/issues/540
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#submit-a-pull-request-review
func (s *PullRequestsService) SubmitReview(owner, repo string, number, reviewID int, review *PullRequestReviewRequest) (*PullRequestReview, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews/%d/events", owner, repo, number, reviewID)

req, err := s.client.NewRequest("POST", u, review)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches
req.Header.Set("Accept", mediaTypePullRequestReviewsPreview)

r := new(PullRequestReview)
resp, err := s.client.Do(req, r)
if err != nil {
return nil, resp, err
}

return r, resp, nil
}

// DismissReview dismisses a specified review on the specified pull request.
//
// TODO: Follow up with GitHub support about an issue with this method's
// returned error format and remove this comment once it's fixed.
// Read more about it here - https://github.com/google/go-github/issues/540
//
// GitHub API docs: https://developer.github.com/v3/pulls/reviews/#dismiss-a-pull-request-review
func (s *PullRequestsService) DismissReview(owner, repo string, number, reviewID int, review *PullRequestReviewDismissalRequest) (*PullRequestReview, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/reviews/%d/dismissals", owner, repo, number, reviewID)

req, err := s.client.NewRequest("PUT", u, review)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches
req.Header.Set("Accept", mediaTypePullRequestReviewsPreview)

r := new(PullRequestReview)
resp, err := s.client.Do(req, r)
if err != nil {
return nil, resp, err
}

// State can be "approved", "rejected", or "commented".
State *string `json:"state,omitempty"`
return r, resp, nil
}
Loading