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

🚧 multiline option #125

Closed
wants to merge 1 commit into from
Closed

🚧 multiline option #125

wants to merge 1 commit into from

Conversation

gr2m
Copy link
Contributor

@gr2m gr2m commented Aug 21, 2020

I've added failing test so we can discuss the way we want to address #97

I would suggest to add a multiline option so we can release this as a feature release, then change the default from false to true as part of v5

eventually fixes #97

}))
log.info({ msg: { b: { c: 'd' } }, extra: { foo: 'bar', number: 42 } })
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing that both the msg and the extra data are logged inline instead of separated by newlines

@gr2m
Copy link
Contributor Author

gr2m commented Aug 21, 2020

We could also do a singleline option and set it to false by default, then set it to true by default with v5.

Or if you prefer the single line altogether, we can just make it the default without adding any option and release that as v5. I think we'll be able to remove quite some code.

@gr2m
Copy link
Contributor Author

gr2m commented Aug 21, 2020

What about error messages?

@mcollina
Copy link
Member

What about error messages?

I think the stack should be on multiple lines.

@gr2m
Copy link
Contributor Author

gr2m commented Aug 22, 2020

Can you tell me what way you want me to implement this?

  1. Add multiline option (or: multiLine?)
  2. Add singleline option (or: singleLine?)
  3. Don't add an option, just change behavior

@jsumners
Copy link
Member

Can I see what the result would look like for a big object? The "deepObject" fixture in the Pino benchmarks should suffice.

@LucasPickering
Copy link
Contributor

@gr2m Any update on this? I'd be happy to work on implementation if you don't have time.

@gr2m
Copy link
Contributor Author

gr2m commented Feb 22, 2021

@gr2m Any update on this? I'd be happy to work on implementation if you don't have time.

no update. Yes, please, it's all yours! Happy to review your own pull request. Sorry that I still didn't find the time to finish this

@jsumners
Copy link
Member

@LucasPickering please see #144 (comment) for instructions on how to include @gr2m's work if you are not familiar.

@LucasPickering LucasPickering mentioned this pull request Feb 22, 2021
2 tasks
@mcollina
Copy link
Member

Close with no activity.

@mcollina mcollina closed this May 26, 2021
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.

Add option for single-line logging of extra attributes
4 participants