pinojs / pino

🌲 super fast, all natural json logger
http://getpino.io
MIT License
14.21k stars 875 forks source link

readme mixin doc: wrong sample code #951

Closed hraban closed 3 years ago

hraban commented 3 years ago

The section on mixins in the readme has the following code:

const mixin = {
    appName: 'My app'
}

const logger = pino({
    mixin(obj) {
        return {
          description: obj.description
        }
    }
})

pino.info({
    description: 'Ok'
}, 'Message 1')
// {"level":30,"time":1591195061437,"pid":16012,"hostname":"x","appName":"My app","description":"Ok" "msg":"Message 1"}
pino.info('Message 2')
// {"level":30,"time":1591195061437,"pid":16012,"hostname":"x","appName":"My app","description":"Ok","msg":"Message 2"}
// Note: the second log contains "description":"Ok" text, even if it was not provided.

but (after fixing the last two pino references to logger), the prophecy in the logs doesn't actually come to be.

What was this bit of code intended to illustrate?

https://github.com/pinojs/pino/blob/master/docs/api.md#mixin-function

jsumners commented 3 years ago

https://github.com/pinojs/pino/pull/857

hraban commented 3 years ago

Interesting. That snippet made sense as merged in that PR, but has since been edited to lose its intended meaning.

mcollina commented 3 years ago

I think we made a mistake in https://github.com/pinojs/pino/pull/928. Would you like to send a PR to fix?

hraban commented 3 years ago

PR: #952

github-actions[bot] commented 2 years ago

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