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

fix: test for multiple caller entries #1304

Merged
merged 3 commits into from
Jan 20, 2022
Merged

fix: test for multiple caller entries #1304

merged 3 commits into from
Jan 20, 2022

Conversation

moshie
Copy link
Contributor

@moshie moshie commented Jan 19, 2022

Fixes: #1303

@mcollina
Copy link
Member

Can you please add a unit test?

@mcollina
Copy link
Member

Does it fix your problem?

@moshie
Copy link
Contributor Author

moshie commented Jan 19, 2022

yes it does 👍

@moshie
Copy link
Contributor Author

moshie commented Jan 20, 2022

I was just looking at the documentation and noticed that the docs are specifying that caller is an option:
https://getpino.io/#/docs/api?id=pinotransportoptions-gt-threadstream

Here I have changed it to callers this may end up breaking some people's implementations.

I will update the implementation to accept caller and wrap it in an array if it's provided otherwise default to the getCallers fn

@moshie
Copy link
Contributor Author

moshie commented Jan 20, 2022

@mcollina This should be ready for a review whenever you're ready 👍

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 6030b26 into pinojs:master Jan 20, 2022
@moshie moshie deleted the pnpm-transform-support branch January 20, 2022 17:13
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pnpm and Pino Transports
2 participants