Skip to content

stream: preserve object mode in compose#47413

Closed
rluvaton wants to merge 1 commit into
nodejs:mainfrom
rluvaton:preserve-object-mode-in-compose
Closed

stream: preserve object mode in compose#47413
rluvaton wants to merge 1 commit into
nodejs:mainfrom
rluvaton:preserve-object-mode-in-compose

Conversation

@rluvaton

@rluvaton rluvaton commented Apr 5, 2023

Copy link
Copy Markdown
Member

fix #46829

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 5, 2023
@debadree25 debadree25 added stream Issues and PRs related to the stream subsystem. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 5, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 5, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@rluvaton

rluvaton commented Apr 5, 2023

Copy link
Copy Markdown
Member Author

@debadree25 Hey man, do you maybe know where to look to try fixing this flaky CI once and for all, it's exhausting

@debadree25

debadree25 commented Apr 5, 2023

Copy link
Copy Markdown
Contributor

Hi @rluvaton this is common occurrence, the failures don't seem related, dont worry I will resume the ci again! in case you are interested in helping in ci flakes you could check out the issue tracker filtering by the flaky-test label! you can also checkout nodejs/reliability repo for common occurences and maybe file issue reports/fix them

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@benjamingr benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This need a CITGM run before merging and probably a semver-major tag

@benjamingr benjamingr added semver-major PRs that contain breaking changes and should be released in the next major version. needs-citgm PRs that need a CITGM CI run. labels Apr 5, 2023
@rluvaton

rluvaton commented Apr 6, 2023

Copy link
Copy Markdown
Member Author

[...] probably a semver-major tag

If compose is experimental, why do we need major?

@mcollina

mcollina commented Apr 8, 2023

Copy link
Copy Markdown
Member

No need for major

@mcollina mcollina added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Apr 8, 2023

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@rluvaton

rluvaton commented Apr 9, 2023

Copy link
Copy Markdown
Member Author

@mcollina Let's merge this think as it's being approved for more than 2 days 😄

@debadree25 debadree25 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 9, 2023
@debadree25

debadree25 commented Apr 9, 2023

Copy link
Copy Markdown
Contributor

this ${STATUS_LABEL} job randomly showing up in github CI as a red, would this conflict with landing? also i think a citgm run would be needed?

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@benjamingr

Copy link
Copy Markdown
Member

queued citgm https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3149/

@rluvaton

rluvaton commented Apr 11, 2023

Copy link
Copy Markdown
Member Author

What is CITGM? Is it testing using other npm packages? and it looks like it failed...

@debadree25

Copy link
Copy Markdown
Contributor

Landed in 67fdb74

@debadree25 debadree25 closed this Apr 11, 2023
@rluvaton rluvaton deleted the preserve-object-mode-in-compose branch April 12, 2023 18:51
@rluvaton

Copy link
Copy Markdown
Member Author

The CITGM looks good (the failures are unrelated, sorry for being uncommunciative @rluvaton !) this can land from my perspective

All good, thanks

I love contributing to node! If there are other stream related issues don't hesitate to mention me 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. semver-minor PRs that contain new features and should be released in the next minor version. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream.compose does not preserve 'readableObjectMode' from final stream to returned stream

6 participants