-
-
Notifications
You must be signed in to change notification settings - Fork 5
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: github status webhooks #27
Conversation
ad14976
to
3373926
Compare
return false; | ||
}; | ||
|
||
app.post('/github-status', async (req, res) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't just expose a plain endpoint for anyone to be able to post stuff to, so we'll need some way to put a secret in the URL. We can either do that here, or I can try to cook something up on the kubernetes side instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woops that's actually a good point. I was assuming you could just make assumptions about the origin, but it's quite likely that's not easily possible.
app.use(express.json()); | ||
app.use('/webhooks', webhooks); | ||
app.listen(8080, () => { | ||
console.log('Bot listening on port 8080'); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be good to do this before bot.login but it probably doesn't matter. also not a bad idea to listen on a port via env, but also probably doesn't matter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll move it up real quick, good idea. Keeping the port hard coded should really be fine in this use case imo.
No description provided.