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

Added documentation for MultiStreamRes, StreamEntry, DestinationStream and Level #1342

Merged
merged 2 commits into from Feb 17, 2022

Conversation

dbacarel
Copy link
Contributor

Should solve #1327

This PR introduces 2 top-level sections in the documentation: Interfaces and Types
Each describes respectively the interfaces and types involved in the object returned by pino.multistream() which is otherwise undocumented.

Also:

  • Added UTs for pino.multistream().add in test/multistream.test.js
  • Added type test for pino.multistream().add in test/multistream.test.js
  • pino.multistream and pino.multistream.add now throw Error if provided with an invalid stream object
  • Minor related type alignments

@github-actions
Copy link

⚠️ This pull request does not target the master branch.

@dbacarel dbacarel changed the title Added documentation about MultiStreamRes, StreamEntry, DestinationStr… Added documentation for MultiStreamRes, StreamEntry, DestinationStream and Level Feb 16, 2022
: typeof dest.level === 'number'
? dest.level
// Default fallback level
: DEFAULT_INFO_LEVEL
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind to restore the previous if/else block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

of course, I restored the if/else block instead of the conditional operator - unless you meant reverting the whole change?

@mcollina
Copy link
Member

@jsumners your action did not work :(

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

@jsumners
Copy link
Member

@jsumners your action did not work :(

I see 😭

@mcollina mcollina merged commit 5d8eb71 into pinojs:master Feb 17, 2022
@dbacarel dbacarel deleted the multistream-add branch February 17, 2022 17:20
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants