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

[v13.x backport] stream: simplify Writable.write #32162

Closed

Conversation

ronag
Copy link
Member

@ronag ronag commented Mar 9, 2020

PR-URL: #31146, #31624

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@ronag ronag added the stream Issues and PRs related to the stream subsystem. label Mar 9, 2020
@ronag ronag requested a review from MylesBorins March 9, 2020 22:16
@ronag
Copy link
Member Author

ronag commented Mar 9, 2020

needs CI, Jenkins is however going down at the moment

@ronag
Copy link
Member Author

ronag commented Mar 9, 2020

Note this actually backports two separate PR's. It was easier like this...

@nodejs-github-bot
Copy link
Collaborator

@MylesBorins
Copy link
Contributor

@ronag generally for backports we want to, as much as possible, keep the original commit meta data in place. In this case we have 3 commits that have now been merged into a single commit. Would it be a ton of extra work to retain that original structure? It makes it harder, at least for me, to review and compare the specific changes to see what the differences are... and almost requires full review now as it is a compeltely different patch then anything that we've landed

@ronag
Copy link
Member Author

ronag commented Mar 10, 2020

@MylesBorins in this specific case it’s a bit harder. Since a commit both of these depend on seems to be missing. I can try again...

@ronag ronag closed this Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants