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

pino v7 transport #36

Merged
merged 7 commits into from
Sep 8, 2021
Merged

pino v7 transport #36

merged 7 commits into from
Sep 8, 2021

Conversation

Eomm
Copy link
Contributor

@Eomm Eomm commented Sep 3, 2021

Ref pinojs/pino#1056

Adding support for pino transport v7: https://github.com/pinojs/pino/blob/master/docs/transports.md

I'm trying to keep the pino legacy transport support: is this needed or this PR may support new transport interface only?

This PR is still draft, but feel free to suggest tests or anything else 👍🏽

@Eomm
Copy link
Contributor Author

Eomm commented Sep 6, 2021

cc @mcollina @davidmarkclements

@mcollina
Copy link
Member

mcollina commented Sep 7, 2021

I'm trying to keep the pino legacy transport support: is this needed or this PR may support new transport interface only?

It is needed.

@mcollina
Copy link
Member

mcollina commented Sep 7, 2021

Good work! What is missing? A lot of tests are failing.

@Eomm Eomm marked this pull request as ready for review September 8, 2021 09:00
psock.js Outdated Show resolved Hide resolved
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 September 8, 2021 11:02
Readme.md Outdated Show resolved Hide resolved
psock.js Outdated Show resolved Hide resolved
Co-authored-by: James Sumners <james@sumners.email>
@Eomm
Copy link
Contributor Author

Eomm commented Sep 8, 2021

As a warning, the CI did not run

1 workflow awaiting approval

Tests are failing for Node<=12 // fixed

@mcollina mcollina merged commit d407483 into pinojs:master Sep 8, 2021
@mcollina mcollina mentioned this pull request Sep 27, 2021
13 tasks
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.

3 participants