Skip to content
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

Project: Extract events logic from @octokit/webhooks into @octokit/events #474

Open
gr2m opened this issue Feb 17, 2021 · 3 comments
Open
Labels
project Goes beyond a single bug fix or new feature. Help welcome by existing contributors.
Projects

Comments

@gr2m
Copy link
Contributor

gr2m commented Feb 17, 2021

Not all GitHub Events are sent via webhooks. Some are only available to GitHub Actions, such as the schedule event.

I would like to move all event-related logic into a new @octokit/events package. It will essentially be EventHandler class from /src/event-handler

@octokit/webhooks should only care about webhooks-specific logic

  1. sign an event payload using a secret
  2. verifying an event payload with a signature
  3. Handle http request lifecycles

With that change, we should rename the @octokit/webhooks-definitions to @octokit/events-defintions, too. The definitions should include a flag whether an event is also sent as a Webhook request by GitHub.

This is also a good opportunity to clean up the code base. @octokit/webhooks is one of the olders @octokit modules, a lot has change since, most importantly the rewrite to TypeScript. When creating @octokit/events, we can start with a blank page and do not need to worry about breaking any APIs. Once the module is done, we will be able to remove a big chunk of the code in @octokit/webhooks and instead replace it with the @octokit/events dependency. We can release a breaking change for @octokit/webhooks if necessary at this point.

This will be a larger effort that I need to be involved in, but it's not time critical. If anyone would like to take a lead on the effort, let's discuss in the comments

@gr2m gr2m added the project Goes beyond a single bug fix or new feature. Help welcome by existing contributors. label Feb 17, 2021
@ghost ghost added this to Inbox in JS Feb 17, 2021
@gr2m gr2m moved this from Inbox to Roadmap in JS Mar 1, 2021
@wolfy1339
Copy link
Member

Can you create the repository so we can plan things there and send an initial implementation

@oscard0m
Copy link
Member

oscard0m commented Jun 5, 2021

I would like to get involved if possible :)

@gr2m
Copy link
Contributor Author

gr2m commented Aug 1, 2021

important note: it seems that not only the events but also the payload schemas may differ between webhooks and GitHub Actions events. I found that while the push webhook event includes commits[][added]/commits[][modified]/commits[][removed] the same properties are not present when handling a push event in GitHub Actions. I've contacted support to find out if this is a mistake or intentionally, but I think it's something we should consider when creating @octokit/events

Update: the payload difference between actions and webhook is indeed by design:

image

@ghost ghost moved this from Roadmap to Inbox in JS Jan 13, 2023
@wolfy1339 wolfy1339 moved this from Inbox to Roadmap in JS Jan 13, 2023
@ghost ghost moved this from Roadmap to Inbox in JS Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project Goes beyond a single bug fix or new feature. Help welcome by existing contributors.
Projects
Status: 🔥 Backlog
JS
  
Inbox
Development

No branches or pull requests

4 participants