V5 docs #447

davidmarkclements · 2018-07-10T23:46:25Z

Apologies if the following explanation of changes/goals is dense, my tired brain has started to give up:

brings documentation inline with any api changes
bitesize reading through subject isolation
configure docsify with pino shades of green & other minor aesthetic adjustments
add pino tree to side bar
restructure api.md for (hopefully) improved clarity
use headers for API for easier linking to specific areas
restructure sidebar according to content changes
split out FAQ into Web Frameworks and Help
make the readme as focused as possible, navigation consistent with sidebar
reduce explanation footprints by removing slightly-off subject content, and/or replacing content with references using internal linking
focus on content ordering that makes most sense for quick referencing and learning through linear reading
consistent formatting
consistent pseudo-code notation (e.g. fn([param]) vs fn(param?) etc)
consistent markdown syntax

Todo

"Migration & Legacy" doc
re-read and apply corrections
make sure all links are working
make sure all external links are tagged with ⇗
make sure all heading levels are correct
probably go over the statics section again as it was done at the high tiredness levels
update benchmarks doc

jsumners · 2018-07-11T01:09:52Z

This is a lot. I'll have to review over the weekend.

mcollina · 2018-07-11T10:11:27Z

README.md


 ## Install

 ```
-npm install pino --save
+$ npm install pino --save


I think we can omit --save now.

mcollina · 2018-07-11T10:11:52Z

README.md

 ```

-If you need support for Node.js v0.12 or v0.10, please install the latest 2.x release using the `legacy` tag:


We need to add a section on pino v4 and pino v2 somewhere.

– todo: add docs/migration-and-legacy.md

mcollina · 2018-07-11T10:13:54Z

docs/api.md

+The options object may additionally contain a `prettifier` property to define
+which prettifier module to use. When not present, `prettifier` defaults to
+`'pino-pretty'`. Regardless of the value, the specified prettifier module
+must be installed as a separate dependency. 


You should add an npm install pino-pretty line here.

mcollina · 2018-07-11T10:14:37Z

docs/api.md

-<a id="metadata"></a>
-# Metadata
+* See [Extreme mode ⇗](extreme.md).
+* See [`sonic-boom` ⇗](https://github.com/mcollina/sonic-boom)


You should also point to pino-multi-stream

hold on I think you've read the removed lines - this isn't the metadata section (that was moved under destination option), this is the pino-extreme secion. I've added a link to pino-multi-stream from there instead

this has been address https://github.com/pinojs/pino/pull/447/files#diff-0f426376b964e609c5c4618eb8bed3b2R227 and https://github.com/pinojs/pino/pull/447/files#diff-0f426376b964e609c5c4618eb8bed3b2R245

mcollina · 2018-07-11T10:15:20Z

docs/web.md

+  logger: true
+})
+fastify.get('/', async (request, reply) => {
+  reply.type('application/json').code(200)


this line is redundant and not needed.

mcollina · 2018-07-11T10:16:14Z

docs/web.md

+
+```js
+const fastify = require('fastify')({
+  logger: true


you can add a comment about passing the logging options to fastify logger: { opts }.

Note that fastify 1 will still be using Pino v4, and we will move to Pino v5 in fastify 2.

ok so I'll add two additional notes underneath the code I think

this is addressed https://github.com/pinojs/pino/pull/447/files#diff-f7e8c7a96957c8be15b0c902dda2d07aR30 and https://github.com/pinojs/pino/pull/447/files#diff-f7e8c7a96957c8be15b0c902dda2d07aR33

mcollina · 2018-07-11T10:16:40Z

docs/web.md

+    }
+  })
+})
+```


We should use the latest example in hapi-pino that covers Hapi v17.

mcollina · 2018-07-11T10:16:55Z

docs/web.md

+server.listen(3000)
+```
+
+See the [restify-pino-logger readme](http://npm.im/restify-pino-logger) for more info.


Does this work with latest restify?

I don't know, need to check. If not we'll need to update restify-pino-logger.

In any case, this won't affect the docs

of course it would affect the docs what am I talking about.

anyway, it does, the syntax since restify v4 is largely unchanged for initialization

updated dev dep in restify-pino-logger from v4 to v7, tests pass and example works

pinojs/restify-pino-logger@343930c

davidmarkclements · 2018-07-14T18:49:23Z

docs/extreme.md

@@ -67,3 +67,6 @@ buffering.
 In case a synchronous flush is needed, `dest.flushSync()` can be called.


it seems to me that the logger.flush method is a footgun then, since logger.flush calls flushSync and not flush - with no warning about data loss in the docs.

Should we remove logger.flush

OR should we rename it to logger.flushSync

OR should we document the confusing nature of it

@mcollina @jsumners

Can you check how much of this is done in #423? I’m definitely +1 to rename it to flushSync, and maybe also have a flush that’s async.

jsumners · 2018-07-15T17:49:47Z

docs/extreme.md

-  `process.exit(1)` otherwise. If you do supply an `onTerminated` function, it
-  is left up to you to fully terminate the process.
+  `process.exit(1)` otherwise. If an `onTerminated` function is supplied, it 
+  us the responsibility of the `onTerminated` function to manually exit the process.


us => is

jsumners · 2018-07-15T17:52:09Z

docs/help.md

+* [Pino with `debug`](#debug)
+
+<a id="rotate"></a>
+## Log rotation


Should we have a link in here somewhere to https://www.nearform.com/blog/sematext-guest-post-pino-fastest-node-js-logger-production/ with a specific mention of using logagent?

we don't link out to blog posts in general.. I think that will become a burden to maintain over time

jsumners

LGTM

I haven't been able to pore over every word, but it looks great.

mcollina

LGTM

github-actions · 2022-02-04T00:04:10Z

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.

v5 docs

bb31ed7

davidmarkclements force-pushed the v5-docs branch from e979880 to bb31ed7 Compare July 10, 2018 23:49

davidmarkclements requested review from jsumners and mcollina July 10, 2018 23:52

davidmarkclements added the v5 label Jul 10, 2018

customLevels docs

69c98e4

mcollina reviewed Jul 11, 2018

View reviewed changes

davidmarkclements mentioned this pull request Jul 13, 2018

v5: custom levels #449

Merged

davidmarkclements added 5 commits July 14, 2018 17:14

remove all --save flags from docs

dd2f9a6

tweaks, legacy.md outline

8866e6b

legacy and migration doc

154110f

corrections

a4c92ab

header level corrections

6faecd4

davidmarkclements commented Jul 14, 2018

View reviewed changes

jsumners reviewed Jul 15, 2018

View reviewed changes

jsumners approved these changes Jul 15, 2018

View reviewed changes

davidmarkclements added 2 commits July 15, 2018 23:06

rm addLevel

9e2854a

typo

2439cda

davidmarkclements mentioned this pull request Jul 15, 2018

feat: flush & flushSync #452

Closed

mcollina approved these changes Jul 16, 2018

View reviewed changes

davidmarkclements added 2 commits July 18, 2018 12:30

bench doc updater

86f341e

flush and flushSync

f4683d2

davidmarkclements force-pushed the v5-docs branch from 445005d to f4683d2 Compare July 18, 2018 11:20

davidmarkclements merged commit 1c7bb0f into next-major Jul 18, 2018

jsumners deleted the v5-docs branch May 25, 2019 13:51

github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V5 docs #447

V5 docs #447

davidmarkclements commented Jul 10, 2018 •

edited

Loading

jsumners commented Jul 11, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

davidmarkclements Jul 14, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 11, 2018

davidmarkclements Jul 14, 2018

davidmarkclements Jul 14, 2018

davidmarkclements Jul 14, 2018

mcollina Jul 15, 2018

jsumners Jul 15, 2018

jsumners Jul 15, 2018

davidmarkclements Jul 15, 2018

jsumners left a comment

mcollina left a comment

github-actions bot commented Feb 4, 2022

		```

		If you need support for Node.js v0.12 or v0.10, please install the latest 2.x release using the `legacy` tag:

		@@ -67,3 +67,6 @@ buffering.
		In case a synchronous flush is needed, `dest.flushSync()` can be called.

V5 docs #447

V5 docs #447

Conversation

davidmarkclements commented Jul 10, 2018 • edited Loading

Todo

jsumners commented Jul 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsumners left a comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 4, 2022

davidmarkclements commented Jul 10, 2018 •

edited

Loading