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

feat: update @octokit/webhooks to v9 #1559

Merged
merged 35 commits into from Jun 23, 2021
Merged

feat: update @octokit/webhooks to v9 #1559

merged 35 commits into from Jun 23, 2021

Conversation

gr2m
Copy link
Contributor

@gr2m gr2m commented Jun 18, 2021

follow up to #1481

wolfy1339 and others added 27 commits June 18, 2021 09:18
BREAKING CHANGE: remove '*' event
BREAKING CHANGE: app.webhooks.middleware has been removed in `@octokit/webhooks` v9
This also makes it comply with the types expected from the `receive()` function from `@octokit/webhooks`
BREAKING CHANGE: removes the `webhookPath` option on `new Probot({})` for the webhooks middleware
…ally a breaking change, if the `x-hub-signature-256` header is not present, the `webhooks.onError()` callback is not called, because it does not even get that far. But I do not think that it is worth mentioning
…les`, add direct dependency to `@octokit/webhooks-types`
@gr2m gr2m requested a review from a team as a code owner June 18, 2021 23:48
@gr2m gr2m changed the base branch from master to beta June 18, 2021 23:49
WebhookExamples.filter(
(event) => event.name === "pull_request"
)[0] as WebhookDefinition<"pull_request">
).examples[0];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are unrelated style changes

@gr2m
Copy link
Contributor Author

gr2m commented Jun 19, 2021

The TypeScript errors when building are gone via 1d49aa1, do you see any problems in the changes? @wolfy1339

I tested it locally and IntelliSense seems to be working as expected

The errors are now signature mismatch errors

@wolfy1339
Copy link
Collaborator

The signature mismatch errors should be gone with the new update of @octokit/webhooks

package.json Outdated Show resolved Hide resolved
@wolfy1339
Copy link
Collaborator

The TypeScript errors when building are gone via 1d49aa1, do you see any problems in the changes? @wolfy1339

All seems good for that change.

@wolfy1339
Copy link
Collaborator

I found some usages of the verify() function where it passes an object instead of a string

diff --git a/test/run.test.ts b/test/run.test.ts
index 941ac54..6610278 100644
--- a/test/run.test.ts
+++ b/test/run.test.ts
@@ -80,10 +80,11 @@ describe("run", () => {
   });
 
   describe("webhooks", () => {
+    const pushEvent = require("./fixtures/webhook/push.json");
     it("POST /", async () => {
       server = await run(() => {}, { env });
 
-      const dataString = require("./fixtures/webhook/push.json");
+      const dataString = JSON.stringify(pushEvent);
 
       await request(server.expressApp)
         .post("/")
@@ -105,7 +106,7 @@ describe("run", () => {
         },
       });
 
-      const dataString = require("./fixtures/webhook/push.json");
+      const dataString = JSON.stringify(pushEvent);
 
       await request(server.expressApp)
         .post("/custom-webhook")

@gr2m gr2m self-assigned this Jun 22, 2021
@gr2m
Copy link
Contributor Author

gr2m commented Jun 22, 2021

This is ready to go now from my side @wolfy1339.

I'll do a follow up PR to address 5774187

@gr2m

This comment has been minimized.

@wolfy1339
Copy link
Collaborator

LGTM 👍

@gr2m gr2m merged commit 5fa615b into beta Jun 23, 2021
@gr2m gr2m deleted the octokit-webhooks-v8 branch June 23, 2021 03:12
@github-actions
Copy link

🎉 This PR is included in version 12.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

gr2m added a commit that referenced this pull request Jun 24, 2021
BREAKING CHANGE: remove '*' event
BREAKING CHANGE: app.webhooks.middleware has been removed in `@octokit/webhooks` v9
BREAKING CHANGE: removes the `webhookPath` option on `new Probot({})` for the webhooks middleware

Co-authored-by: wolfy1339 <webmaster@wolfy1339.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants