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: adds support for printing number and boolean messageKey value type #434

Merged
merged 5 commits into from Jul 4, 2023

Conversation

rjmohammad
Copy link
Contributor

@rjmohammad rjmohammad commented Jun 23, 2023

closes #433

closes: Pino Pretty does not output numbers but only strings. pinojs#433
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 requested a review from jsumners June 25, 2023 11:19
test/basic.test.js Outdated Show resolved Hide resolved
test/basic.test.js Outdated Show resolved Hide resolved
test/basic.test.js Outdated Show resolved Hide resolved
cb()
}
}))
log.info(true)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder what log.info(0) or log.info(1) would produce here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a possible fix for 0.

What is the expectation for log.info(null) or log.info(undefined)?

Copy link
Member

Choose a reason for hiding this comment

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

Only null is valid json. So I imagine we'd just print the word "null". But that can be a separate PR.

rjmohammad and others added 4 commits June 25, 2023 11:31
Co-authored-by: James Sumners <james@sumners.email>
Co-authored-by: James Sumners <james@sumners.email>
Co-authored-by: James Sumners <james@sumners.email>
@jsumners jsumners merged commit c32de73 into pinojs:master Jul 4, 2023
10 checks passed
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.

Pino Pretty does not output numbers but only strings.
3 participants