Skip to content

refactor: pass CancellationToken to WebhookEventProcessor by default #716

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

Merged
merged 1 commit into from
Jul 18, 2025

Conversation

JamieMagee
Copy link
Member

Follow-up from #708 in preparation for a major version release.


Before the change?

  • CancellationToken was an opt-in for Octokit.Webhooks.AspNetCore
  • It was not possible to pass CancellationToken in Octokit.Webhooks.AzureFunctions

After the change?

  • CancellationToken is passed by default in Octokit.Webhooks.AspNetCore and Octokit.Webhooks.AzureFunctions

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

Copy link

👋 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 Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@JamieMagee JamieMagee enabled auto-merge (squash) July 18, 2025 03:51
@JamieMagee JamieMagee moved this from 🆕 Triage to 👀 In review in 🧰 Octokit Active Jul 18, 2025
@JamieMagee JamieMagee changed the title Pass CancellationToken to WebhookEventProcessor by default refactor: pass CancellationToken to WebhookEventProcessor by default Jul 18, 2025
Copy link
Contributor

@sommmen sommmen left a comment

Choose a reason for hiding this comment

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

LGTM.

I'm not familiar with azure functions, but the CTS is also injected;

CancellationToken

So you could use that straight up instead of getting in from the context, but i would assume they would be the same CTS.

@JamieMagee JamieMagee disabled auto-merge July 18, 2025 14:31
@JamieMagee JamieMagee enabled auto-merge (squash) July 18, 2025 14:31
@JamieMagee JamieMagee force-pushed the cancellation-token-default branch from bfedaaf to bb254a2 Compare July 18, 2025 15:48
@JamieMagee JamieMagee merged commit 8b39688 into main Jul 18, 2025
8 checks passed
@JamieMagee JamieMagee deleted the cancellation-token-default branch July 18, 2025 15:52
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in 🧰 Octokit Active Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants