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

fix write call in unit tests #244

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Conversation

berndfuhrmann
Copy link
Contributor

@berndfuhrmann berndfuhrmann commented Sep 27, 2024

I tried updating the package readable-stream to 4.5.2 and noticed that some unit tests are failing. That is because transport.write is called with additional parameters from forEach.
forEach passes three parameters to its callback (MDN): element, index and array. write should only receive element, but previously, it also received index as enc and array as callback.
This PR changes this, so that only element is provided.

I would like to see readable-stream updated and can provide another PR for that. I think dependabot already tried that and failed, maybe because the tests failed.

@DABH
Copy link
Contributor

DABH commented Sep 27, 2024

If you are able to get readable-stream updated in winston-transport, winston, and logform (is it used there? maybe not), that would be incredible 😍 Thank you so much for the contribution! Will try to review any PRs you open

Copy link
Contributor

@DABH DABH left a comment

Choose a reason for hiding this comment

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

This change seems pretty straightforward and logical to me, I'm happy to give my +1. Thank you!

@DABH DABH merged commit fb8383e into winstonjs:master Sep 27, 2024
3 checks passed
@berndfuhrmann
Copy link
Contributor Author

@DABH Thanks for merging this and releasing a new version. Working on updating Winston with the new version.

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.

2 participants