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 the ability to build a transport using a transform #25

Merged
merged 7 commits into from
Oct 2, 2021
Merged

Conversation

mcollina
Copy link
Member

No description provided.

@mcollina
Copy link
Member Author

cc @Eomm @jsumners this will be the basis for the pino-socket/pino-syslog issue.

@coveralls
Copy link

coveralls commented Sep 29, 2021

Pull Request Test Coverage Report for Build 1290626233

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 1280778809: 0.0%
Covered Lines: 42
Relevant Lines: 42

💛 - Coveralls

README.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -97,6 +101,40 @@ module.exports = function (opts) {
}
```

### Stream concatenation / pipeline

You can pipeline multiple transport as well.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
You can pipeline multiple transport as well.
You can pipeline multiple transports:

It would be good to link to a concrete (real world) example in the pino-syslog docs when it is implemented.

Copy link
Member Author

Choose a reason for hiding this comment

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

I Agree. I'll update the doc once this is implemented fully.

mcollina and others added 2 commits September 29, 2021 20:13
Co-authored-by: James Sumners <[email protected]>
Co-authored-by: James Sumners <[email protected]>
README.md Outdated Show resolved Hide resolved
mcollina and others added 2 commits September 30, 2021 10:10
@@ -51,8 +51,8 @@ Create a [`split2`](http://npm.im/split2) instance and returns it.
This same instance is also passed to the given function, which is called
synchronously.

If `fn` returns a [`Readable`](https://nodejs.org/api/stream.html#stream_class_stream_readable), we will
wrap that readable and the split2 instance using [`duplexify`](https://www.npmjs.com/package/duplexify),
If `opts.transform` is `true`, `pino-abstract-transform` will
Copy link
Member

Choose a reason for hiding this comment

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

opts.enablePipelining?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If `opts.transform` is `true`, `pino-abstract-transform` will
If `opts.enablePipelining` is `true`, `pino-abstract-transform` will

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If `opts.transform` is `true`, `pino-abstract-transform` will
If `opts.enablePipelining` is `true`, `pino-abstract-transform` will

index.js Outdated Show resolved Hide resolved
@@ -51,8 +51,8 @@ Create a [`split2`](http://npm.im/split2) instance and returns it.
This same instance is also passed to the given function, which is called
synchronously.

If `fn` returns a [`Readable`](https://nodejs.org/api/stream.html#stream_class_stream_readable), we will
wrap that readable and the split2 instance using [`duplexify`](https://www.npmjs.com/package/duplexify),
If `opts.transform` is `true`, `pino-abstract-transform` will
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If `opts.transform` is `true`, `pino-abstract-transform` will
If `opts.enablePipelining` is `true`, `pino-abstract-transform` will

Co-authored-by: Manuel Spigolon <[email protected]>
@mcollina mcollina merged commit de2ca57 into main Oct 2, 2021
@mcollina mcollina deleted the transform branch October 2, 2021 21:47
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.

4 participants