-
Notifications
You must be signed in to change notification settings - Fork 102
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
Add support for handling review comments #257
Conversation
Thanks for your interest in palantir/policy-bot, @jgiannuzzi! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
@@ -548,7 +548,12 @@ func (ghc *GitHubContext) loadPagedData() error { | |||
} | |||
|
|||
for _, r := range q.Repository.PullRequest.Reviews.Nodes { | |||
reviews = append(reviews, r.ToReview()) | |||
switch r.State { | |||
case "COMMENTED": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be case string(githubv4.PullRequestReviewStateCommented):
instead?
I did not find any instance of using the githubv4
module constants anywhere in the code, and thus decided to use these strings instead, but happy to change that if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to keep these as strings for now
switch r.State { | ||
case "COMMENTED": | ||
comments = append(comments, r.ToComment()) | ||
case "APPROVED", "CHANGES_REQUESTED": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing this and updating the tests!
@@ -548,7 +548,12 @@ func (ghc *GitHubContext) loadPagedData() error { | |||
} | |||
|
|||
for _, r := range q.Repository.PullRequest.Reviews.Nodes { | |||
reviews = append(reviews, r.ToReview()) | |||
switch r.State { | |||
case "COMMENTED": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to keep these as strings for now
Hopefully we can include this in a release by the end of the week; there's one other feature currently in progress that I'd like to complete first. For now, you can test with the |
That would be awesome, thanks! |
This PR adds support for handling review comments like regular comments, thus allowing users to approve or disapprove without switching back to the "Conversation" tab of the pull request whilst reviewing the changes.
Fixes #51.