feat: allow to specify timeout option in middleware, refactor middleware
#1148
Conversation
|
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with |
|
Is this now acceptable? |
There was a problem hiding this comment.
Looks good to me. I would suggest to do a new release due to the significance of the changes. It looks like we should make this a feature release since timeout is now an argument for the Node.js middleware. Should we document it at https://github.com/octokit/webhooks.js?tab=readme-ov-file#createnodemiddleware, or is that just exposed for testing?
wolfy1339
changed the title
timeout option in middleware, refactor middleware
|
THe timeout is not node specific.for Web is also possible to configure a timeout. I created it for testing. I dont think people need to configure the timeout. |
|
Should we merge this first and then the tests or how do we want to proceed? |
|
🎉 This PR is included in version 13.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 13.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
This PR is builds on #1146.
As you can see, the unit test pass.
In a follow up PR we can adapt the new tests I wrote, and then we can test for deno and bun ;)