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

Support Winston as Custom Logger #33

Merged
merged 7 commits into from
Jun 19, 2024
Merged

Support Winston as Custom Logger #33

merged 7 commits into from
Jun 19, 2024

Conversation

CHC383
Copy link

@CHC383 CHC383 commented Jun 11, 2024

Partially fix #29

Description

Fix the code and actually support winston as a custom logger

Testing

  • npm run test
  • npm run lint

@CHC383
Copy link
Author

CHC383 commented Jun 18, 2024

I added two commits, one for moving pino to peerDeps and one for updating the usage instructions to fix #13. Please feel free to merge them or I could separate them into another PR depending on your preference.

I tried to update the tests to use the instrumentation hook so that they align with the new instructions, but I couldn't find a good way as the tests invoke the Next logging and console logging directly instead of running the whole Next stack which will load the instrumentation hook. Let me know if you have any thoughts and I am happy to help update the code.

And these are all of my current plans of changes, if there aren't anything new, I think you could bump up the version (5.x? since moving pino could be considered as a breaking change, requiring the users to explicitly install one if they don't already) and make a new release.

labscale-huacongc and others added 7 commits June 19, 2024 20:09
- Handle trace logging for winston properly
- Add unit tests
- Update README
Add Winston as a dev dependency so that it is installed during development and resolvable by eslint
- Make `pino` a peer dependencies and expand the version range, so that the users could pick their own version without having multiple copies in the bundle.
- Update the README accordingly.
Update the instructions of using the library to use Next Instrumentation hook, which works for differerent cases including run from CLI, run through npm or standalone mode.
@atkinchris atkinchris merged commit 72d63a5 into sainsburys-tech:main Jun 19, 2024
2 checks passed
@atkinchris
Copy link
Collaborator

This is published now as v5.0.0. Thank you @CHC383! 🎉

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.

it fails to import the winston logger.
3 participants