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: Initial webhook-event-check feature #1078

Merged
merged 36 commits into from Dec 19, 2019

Conversation

MaximDevoir
Copy link
Contributor

@MaximDevoir MaximDevoir commented Dec 2, 2019

A hook-check feature will warn developers if they are attempting to listen to an event their GitHub App is not subscribed to.

========

100% unit tested when running yarn run jest --coverage check-event.test.ts.

========

Example warning of an invalid event name (e.g. typo):

warning example

Example warning if an application is not connected to the Internet or bad configuration settings:

event-check no-internet

Limitations

  • Event-check is only capable of checking the base part of an event - the text before the period, and not the action part of the event. For example, a Probot app may want to listen for new issues being opened using app.on('issues.opened, ...). Event-check will warn the user if the GitHub App is not subscribed to the issues event - but will not verify if the action (opened) is valid.

Closes #1042

src/hook-check.ts Outdated Show resolved Hide resolved
src/hook-check.ts Outdated Show resolved Hide resolved
@MaximDevoir MaximDevoir marked this pull request as ready for review December 5, 2019 20:27
@MaximDevoir MaximDevoir requested a review from a team as a code owner December 5, 2019 20:27
@MaximDevoir
Copy link
Contributor Author

Question: I've been thinking about this a bit, should the feature be renamed to webhook-event-check? I renamed it from webhook-check to event-check, but I'm wondering if that is the best name.

@gr2m
Copy link
Contributor

gr2m commented Dec 6, 2019

Naming is hard :) I like webhook-event-check, it's the most explicit.

@gr2m
Copy link
Contributor

gr2m commented Dec 6, 2019

Let us know when this PR is ready for review. If you are still working on it, can you prefix the pull request with [WIP] or 🚧?

@MaximDevoir MaximDevoir changed the title feat: Initial hook-check feature WIP | feat: Initial hook-check feature Dec 6, 2019
@MaximDevoir MaximDevoir changed the title WIP | feat: Initial hook-check feature feat: Initial hook-check feature Dec 6, 2019
@MaximDevoir
Copy link
Contributor Author

OK, I think it's ready for your review.

@MaximDevoir MaximDevoir changed the title feat: Initial hook-check feature feat: Initial webhook-event-check feature Dec 6, 2019
docs/configuration.md Outdated Show resolved Hide resolved
@gr2m gr2m merged commit d8aa4e0 into probot:master Dec 19, 2019
@probotbot
Copy link

🎉 This PR is included in version 9.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gr2m
Copy link
Contributor

gr2m commented Dec 9, 2020

@MaximDevoir fyi, I'll have to remove the feature for now as it become a blocker for our serverless/function milestone. But I created a follow up issue to find another solution: #1435

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.

Warn developer when using un-registered hook
3 participants