-
Notifications
You must be signed in to change notification settings - Fork 897
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
pass mkdir option on pino/file transport to destination #1223
pass mkdir option on pino/file transport to destination #1223
Conversation
6c285cc
to
bc93da6
Compare
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.
Could you add the documentation for this "new" option? Thanks!
Absolutely! Stay tuned.
|
bc93da6
to
5cd6127
Compare
@@ -4,7 +4,9 @@ const pino = require('./pino') | |||
const { once } = require('events') | |||
|
|||
module.exports = async function (opts = {}) { | |||
const destination = pino.destination({ dest: opts.destination || 1, sync: false }) | |||
const destOpts = { dest: opts.destination || 1, sync: false } | |||
if (opts.mkdir) destOpts.mkdir = true |
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.
why not add ternary condition on destOpts 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.
I'm happy to write it any way that's acceptable to the project. Adding an undefined property broke an existing test. I wrote it in such a way that the existing test would pass.
I've added a paragraph with an example to the transports.md docs as requested. Let me know if there's anything you'd like me to change. |
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.
lgtm
@mojavelinux could you send another PR with the |
Yep! On it. |
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. |
If
mkdir
option is specified on the pino/file transport, pass it to the pino.destination.closes #1221