-
Notifications
You must be signed in to change notification settings - Fork 60.3k
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
Do installation
and installation_repository
payloads have the property installation
100% of the times?
#516
Comments
Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines. |
…llation' until this issue is clarified github/docs#516
👋 @dominguezcelada To answer your question:
The
This means that if the event for the webhook involves a GitHub App, it will contain the Because the webhooks Comparing it other webhooks, such as I hope this makes sense! 🙂 |
Has totally sense @lucascosti . Thanks for the clear explanation 💪🏽 I was thinking... would have sense to improve a little bit the way is expressed so, in case of webhooks like
to
only for this kind of events. Do you think it could have sense this improvement? Thanks for your time! |
@dominguezcelada 👍 I think that makes sense. For those endpoints, we could remove the |
Cool! I will be happy to open a PR for this. Should I create a new issue linking to this one? Seems I'm not able to re-open the issue myself. Thanks @lucascosti |
My bad, I forgot that external contributors can't reopen 😊 . Reopened for you 🚀 |
Hello, I'm building my own GitHub App using ProBot and I've a registered app on some private repositories. Does the app required to be configured in a special way in order to receive the installation property as part of the push event payload? |
👋 @Kerruba! This docs repository is not the best place to troubleshoot product issues not directly related to the documentation. Better places to get help would be the GitHub Support Community forums, or contacting GitHub Support. 🙂 |
This comment has been minimized.
This comment has been minimized.
@lucascosti I ended up opening a PR for this after a long time! Would it be possible to re-open this issue? I think it was closed by mistake or by its inactivity |
What article on docs.github.com is affected?
https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/webhook-events-and-payloads
What part(s) of the article would you like to see updated?
https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/webhook-events-and-payloads#installation
https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/webhook-events-and-payloads#installation_repositories
Additional information
docs/content/developers/webhooks-and-events/webhook-events-and-payloads.md
Lines 446 to 448 in d516858
and
docs/content/developers/webhooks-and-events/webhook-events-and-payloads.md
Lines 470 to 472 in 20fa34e
Checking the description of
installation
property, it denotes it could exist or not:docs/data/reusables/webhooks/app_desc.md
Line 1 in d516858
but due to the nature of the events, it should always have
installation
property right?Context:
@octokit/webhook-definition
is working to improve typing of installation property in Payloads so it becomes an optional property (when necessary)The text was updated successfully, but these errors were encountered: