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: log Probot version at startup #1321

Merged
merged 2 commits into from
Aug 28, 2020
Merged

feat: log Probot version at startup #1321

merged 2 commits into from
Aug 28, 2020

Conversation

shaftoe
Copy link
Contributor

@shaftoe shaftoe commented Aug 28, 2020

I'm new to TypeScript so please forgive in advance any silly mistake here.

I believe it's valuable to know which version of Probot a Probot-application instance is based on, AFAIK the only way at the moment is to inspect node_modules/probot/package.json but I believe at least logging it at startup is a good thing.

Probably is also a good idea to expose that via GET /version, together with the actual application version, something like this maybe?

{
  "appVersion": "0.1.2",
  "probotVersion": "3.4.5"
}

@shaftoe shaftoe requested a review from a team as a code owner August 28, 2020 11:23
src/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this!

Logging the version at startup is a good idea. I don't think we need to add a new GET /version endpoint though. Exposing the probot version publicly might be a security risk, if there is ever a known security vulnerability. We do display the app version on GET /probot, e.g. see https://probot-dco.herokuapp.com/probot

test/index.test.ts Outdated Show resolved Hide resolved
test/index.test.ts Outdated Show resolved Hide resolved
@shaftoe
Copy link
Contributor Author

shaftoe commented Aug 28, 2020

Logging the version at startup is a good idea. I don't think we need to add a new GET /version endpoint though. Exposing the probot version publicly might be a security risk, if there is ever a known security vulnerability. We do display the app version on GET /probot, e.g. see https://probot-dco.herokuapp.com/probot

Ok, understood. Out of curiosity, isn't exposing the app version at GET /probot' (actually also GET /` if I get it right) adding a security risk as well?

Other related question: what's the recommended way to test Probot beta/next? npx create-probot-app + npm install probot@next ?

@gr2m
Copy link
Contributor

gr2m commented Aug 28, 2020

isn't exposing the app version [...] adding a security risk as well?

The app version is independent of the probot version the app is using, so I don't think so.

what's the recommended way to test Probot next

npx create-probot-app is already set up to use probot v10

Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

image

@gr2m gr2m merged commit c59fbcc into probot:next Aug 28, 2020
@github-actions
Copy link

🎉 This PR is included in version 10.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@shaftoe shaftoe deleted the add-version branch August 29, 2020 06:35
gr2m pushed a commit that referenced this pull request Sep 1, 2020
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