Add support for apps webhook config endpoints#2096
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2096 +/- ##
=======================================
Coverage 97.77% 97.78%
=======================================
Files 109 110 +1
Lines 9724 9746 +22
=======================================
+ Hits 9508 9530 +22
Misses 150 150
Partials 66 66
Continue to review full report at Codecov.
|
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @rpelliard !
One tiny nit to fix, please, but otherwise LGTM.
Awaiting second LGTM before merging.
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
gmlewis
left a comment
There was a problem hiding this comment.
LGTM.
Awaiting second LGTM before merging.
Co-authored-by: Parker77 <Parker77@users.noreply.github.com>
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
@googlebot I fixed it. |
Thank you, @Parker77 ! |
This adds support for two Apps API endpoints (see #2095)
Both endpoints require authentication as an app (with a JWT), which can be dealt with by the underlying transport.
A few notes:
I reused / extended the
HookConfigstruct which was defined inorgs_audit_logs.go, since it represents the same object.The
InsecureSSLfield of the struct is a*string. However, GitHub only returns"0"or"1"(insecure SSL disabled / enabled), so I think it'd be more user-friendly to use a*bool, and write some logic so that boolean values are correctly (un)marshaled from"0"/"1". I'd be happy to write the code to deal with this, but that would be a breaking change so I wanted to check with you first.This is probably out of scope of this PR, but the
Hookstruct used for repository / organization webhooks endpoints is using amap[string]interface{}for theConfigfield, but I think it could be using theHookConfigstruct as well since it represents the same object. Happy to make that change in a separate PR, but this would be a breaking change as well.