-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 github pkg to webhook #1230
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.
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 171 Continue to review full report at Codecov.
|
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.
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()) |
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.
log.Printf("Succesfully commented on PR %d.", pr.GetNumber()) | |
log.Printf("Successfully commented on PR %d", pr.GetNumber()) |
webhook/webhook.go
Outdated
} | ||
if err := kubernetes.WaitForDeploymentToStabilize(deployment); err != nil { | ||
return errors.Wrapf(err, "waiting for deployment %s to stabilize", deployment.Name) | ||
} | ||
// 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 |
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.
// 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], |
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.
this looks weird... why isn't this just message
?
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.
whoops my bad, Body requires a string pointer and previously I was setting "message" directly in the array. fixed now!
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.