feat(typescript): adds organization to ping event types
#378
Conversation
octokitbot
added
the
Type: Maintenance
gr2m
changed the title
organization to ping event types
| test_url: string; | ||
| test_url?: string; | ||
| ping_url: string; | ||
| last_response: WebhookPayloadPingHookLastResponse; | ||
| last_response?: WebhookPayloadPingHookLastResponse; | ||
| }; | ||
| type WebhookPayloadPing = { | ||
| zen: string; | ||
| hook_id: number; | ||
| hook: WebhookPayloadPingHook; | ||
| repository: PayloadRepository; | ||
| repository?: PayloadRepository; | ||
| sender: WebhookPayloadPingSender; | ||
| organization?: WebhookPayloadPingOrganization; |
There was a problem hiding this comment.
@G-Rath could you please double check if these changes look good to you?
There was a problem hiding this comment.
Yes, they look fine.
Just btw, the ideal would be a union type a la:
type WebhookPayloadPingBase = {
zen: string;
hook_id: number;
hook: WebhookPayloadPingHook;
sender: WebhookPayloadPingSender;
};
interface WebhookPayloadPingOrg extends WebhookPayloadPingBase {
organisation: WebhookPayloadPingOrganization;
}
interface WebhookPayloadPingRepo extends WebhookPayloadPingBase {
repository: PayloadRepository;
}
type WebhookPayloadPing = WebhookPayloadPingOrg | WebhookPayloadPingRepo
As this would properly represent that you can't have both, and you'd do:
if (event.name === 'ping') {
if ('organisation' in event.payload) {
console.log(event.payload.organisation.avatar_url);
}
}
But that'd be a decent refactor of the type generator + the types themselves, and these types are serviceable as they are currently :)
There was a problem hiding this comment.
The current types are generated using https://github.com/gimenete/type-writer in https://github.com/octokit/webhooks.js/blob/84f753b284d1a9fef94465a3463de94d0b63da48/scripts/generate-types.js from the JSON example payloads.
The proper solution would be to create a JSON Schema that will become the single place of truth, and then only use the examples as means of testing the schema:
octokit/webhooks#186
We had a similar evolution happening with the types for @octokit/rest and its REST API endpoint methods. Originally I created an OpenAPI spec myself by scraping GitHub's doc pages, eventually GitHub published an official OpenAPI spec with proper schema, which greatly improved the types and reduces the code size: octokit/types.ts#197 (There is a blog post about the whole process). I'm pretty sure eventually GitHub will publish and maintain official JSON Schemas for all the events. I want us to help in the process by creating a starting point via octokit/webhooks#186, similar to what we did for the REST API endpoints
|
🎉 This PR is included in version 7.17.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
A new release of @octokit/webhooks was just released 👋🤖
This pull request updates the TypeScript definitions derived from
@octokit/webhooks. I can't tell if the changes are fixes, features or breaking, you'll have to figure that out on yourself and adapt the commit messages accordingly to trigger the right release, see our commit message conventions.