-
Notifications
You must be signed in to change notification settings - Fork 8
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(pino): bump pino
dependency
#188
base: main
Are you sure you want to change the base?
Conversation
@gr2m I believe we need to reduce scope of what this package is doing. in v7 it seems like if you want to log to sentry and pino-pretty the ideal way is multiple transports:
right now this package is trying to both log pretty, json and send to sentry 😅 |
I'm open to do multiple transforms instead if that better aligns with how |
also fixes old package-lock.json and security issues in dev dependencies
@gr2m I think I managed to adapt the code to a transformer based on pino-pretty, without needing to split into multiple transformers. Now I just need to adapt the tests 😅 |
example.js is definitely working:
|
I am not sure how to adapt the tests. Any guidance or help would be appreciated @gr2m |
@gr2m potentially we could remove this from probot entirely? This is only for those using sentry and they could hook it up themself inside their applications? https://www.npmjs.com/package/pino-sentry
|
also fixes old package-lock.json and security issues in dev dependencies
Please correct me if I am using semantic commits wrong 😅 I expect this is a feature to upgrade to a newer major version of pino
Made it a draft to use smaller commits and prepare it slowly but keep it ready for CI and reviews.