Add github pkg to webhook#1230
Merged
priyawadhwa merged 3 commits intoGoogleContainerTools:masterfrom Nov 2, 2018
Merged
Conversation
This is the final piece of the webhook, which should allow it to comment on github PRs when the sample websites are ready and remove labels from PRs. I also made some small logging and error handling changes to make them easier to read. I also tested the full flow on a sample PR and it seems to be working as expected.
priyawadhwa
requested review from
balopat,
dgageot,
nkubala and
r2d4
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1230 +/- ##
=======================================
Coverage 41.45% 41.45%
=======================================
Files 96 96
Lines 4238 4238
=======================================
Hits 1757 1757
Misses 2310 2310
Partials 171 171Continue to review full report at Codecov.
|
balopat
approved these changes
Nov 2, 2018
Contributor
balopat
left a comment
balopat
left a comment
There was a problem hiding this comment.
only some nits but otherwise LGTM.
pkg/webhook/github/github.go
Outdated
| if err != nil { | ||
| return errors.Wrap(err, "creating github comment") | ||
| } | ||
| log.Printf("Succesfully commented on PR %d.", pr.GetNumber()) |
Contributor
There was a problem hiding this comment.
Suggested change
| log.Printf("Succesfully commented on PR %d.", pr.GetNumber()) | |
| log.Printf("Successfully commented on PR %d", pr.GetNumber()) |
webhook/webhook.go
Outdated
| } | ||
| // TODO: priyawadhwa@ to add logic for commenting on Github once the deployment is ready | ||
|
|
||
| // Now, comment on the PR and remove the docs-modifications label |
Contributor
There was a problem hiding this comment.
Suggested change
| // Now, comment on the PR and remove the docs-modifications label | |
| // comment on the PR and remove the docs-modifications label |
pkg/webhook/github/github.go
Outdated
| // CommentOnPR comments message on the PR | ||
| func (g *Client) CommentOnPR(pr *github.PullRequestEvent, message string) error { | ||
| comment := &github.IssueComment{ | ||
| Body: &[]string{message}[0], |
Contributor
There was a problem hiding this comment.
this looks weird... why isn't this just message?
Contributor
Author
There was a problem hiding this comment.
whoops my bad, Body requires a string pointer and previously I was setting "message" directly in the array. fixed now!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the final piece of the webhook, which should allow it to comment
on github PRs when the sample websites are ready and remove labels from
PRs.
I also made some small logging and error handling changes to make them
easier to read. I also tested the full flow on a sample PR and it seems to be
working as expected.