-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 App Callback URLs #3204
Conversation
scrape/apps.go
Outdated
@@ -115,6 +115,8 @@ type AppManifest struct { | |||
Name *string `json:"name,omitempty"` | |||
//Required. The homepage of your GitHub App. | |||
URL *string `json:"url,omitempty"` | |||
// The full URL of the endpoint to authenticate users via the GitHub App. | |||
CallbackURL *string `json:"url,omitempty"` |
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 change makes no sense to me, based on line 117 above it.
Note that both fields have identical JSON bindings, which means they get mapped to the same thing:
URL *string `json:"url,omitempty"`
CallbackURL *string `json:"url,omitempty"`
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.
You're absolutely right. I should have copied the code from my test, but didn't.
It's now fixed.
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.
Thank you, @Roming22 !
Could I please trouble you to add the GitHub API Docs URL as a comment to this code that shows where this is documented?
I know this is the scrape
directory, but it would be nice to have a reference to it.
Thanks again!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3204 +/- ##
==========================================
- Coverage 97.72% 92.92% -4.80%
==========================================
Files 153 171 +18
Lines 13390 11582 -1808
==========================================
- Hits 13085 10763 -2322
- Misses 215 726 +511
- Partials 90 93 +3 ☔ View full report in Codecov by Sentry. |
I do not know where, or whether, it is documented. I look at the surrounding fields and made a lucky guess. |
Found it! |
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
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.
Thank you, @Roming22 !
LGTM.
Merging.
Thanks for the support @gmlewis! Is there an ETA for the next release? |
It looks like it is about time. I'll work on it. |
This is now available here: https://github.com/google/go-github/releases/tag/v63.0.0 |
Fixes: #3203