Skip to content

stream: remove always-false condition check#41488

Merged
nodejs-github-bot merged 1 commit intonodejs:masterfrom
Trott:guaranteed
Jan 15, 2022
Merged

stream: remove always-false condition check#41488
nodejs-github-bot merged 1 commit intonodejs:masterfrom
Trott:guaranteed

Conversation

@Trott
Copy link
Member

@Trott Trott commented Jan 12, 2022

Remove comparison to null of variable guaranteed to be a boolean.

@Trott
stream: remove always-false condition check
2dae413 
Remove comparison to null of variable guaranteed to be a boolean.
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 12, 2022

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 12, 2022
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 12, 2022

CI: https://ci.nodejs.org/job/node-test-pull-request/41827/

mcollina
mcollina approved these changes Jan 12, 2022
View reviewed changes
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

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 12, 2022
aduh95
aduh95 reviewed Jan 12, 2022
View reviewed changes
lib/internal/streams/utils.js
@@ -119,14 +119,14 @@ function isReadableFinished(stream, strict) {
function isReadable(stream) {
if (stream && stream[kIsReadable] != null) return stream[kIsReadable];
const r = isReadableNodeStream(stream);
Copy link
Contributor

@aduh95 aduh95 Jan 12, 2022

Choose a reason for hiding this comment

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

r value is unused if typeof stream?.readable is not a boolean, it would be worth move its assignment below. (Same in isWritable)

@Trott Trott 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 Jan 14, 2022
@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 14, 2022
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@Trott Trott removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jan 14, 2022
@VoltrexKeyva VoltrexKeyva removed the needs-ci PRs that need a full CI run. label Jan 14, 2022
@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 14, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 15, 2022
@nodejs-github-bot nodejs-github-bot merged commit f7be6ab into nodejs:master Jan 15, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 15, 2022

Landed in f7be6ab

@Trott Trott deleted the guaranteed branch January 15, 2022 00:27
Trott added a commit to Trott/io.js that referenced this pull request Jan 15, 2022
@Trott
stream: avoid function call where possible
2b443be 
Instead of assigning a boolean, move the function call that assigns a
value to the boolean to the only place that boolean is checked. This
avoids the function call in cases where it is not needed.

Refs: nodejs#41488 (review)
@Trott Trott mentioned this pull request Jan 15, 2022
Merged
targos pushed a commit that referenced this pull request Jan 16, 2022
@Trott @targos
stream: remove always-false condition check
51d86fe 
Remove comparison to null of variable guaranteed to be a boolean.

PR-URL: #41488
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@targos targos mentioned this pull request Jan 16, 2022
Merged
mawaregetsuka pushed a commit to mawaregetsuka/node that referenced this pull request Jan 17, 2022
@Trott @mawaregetsuka
stream: remove always-false condition check
1d49522 
Remove comparison to null of variable guaranteed to be a boolean.

PR-URL: nodejs#41488
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Jan 17, 2022
@Trott
stream: avoid function call where possible
20347d5 
Instead of assigning a boolean, move the function call that assigns a
value to the boolean to the only place that boolean is checked. This
avoids the function call in cases where it is not needed.

Refs: #41488 (review)

PR-URL: #41534
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
thedull pushed a commit to thedull/node that referenced this pull request Jan 18, 2022
@Trott @thedull
stream: remove always-false condition check
9eff406 
Remove comparison to null of variable guaranteed to be a boolean.

PR-URL: nodejs#41488
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
thedull pushed a commit to thedull/node that referenced this pull request Jan 18, 2022
@Trott @thedull
stream: avoid function call where possible
c7e1826 
Instead of assigning a boolean, move the function call that assigns a
value to the boolean to the only place that boolean is checked. This
avoids the function call in cases where it is not needed.

Refs: nodejs#41488 (review)

PR-URL: nodejs#41534
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
BethGriggs pushed a commit that referenced this pull request Jan 25, 2022
@Trott @BethGriggs
stream: avoid function call where possible
9226322 
Instead of assigning a boolean, move the function call that assigns a
value to the boolean to the only place that boolean is checked. This
avoids the function call in cases where it is not needed.

Refs: #41488 (review)

PR-URL: #41534
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
@Trott @Linkgoron
stream: remove always-false condition check
db8e623 
Remove comparison to null of variable guaranteed to be a boolean.

PR-URL: nodejs#41488
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
@Trott @Linkgoron
stream: avoid function call where possible
dfb25ff 
Instead of assigning a boolean, move the function call that assigns a
value to the boolean to the only place that boolean is checked. This
avoids the function call in cases where it is not needed.

Refs: nodejs#41488 (review)

PR-URL: nodejs#41534
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
@Trott @danielleadams
stream: remove always-false condition check
5e890be 
Remove comparison to null of variable guaranteed to be a boolean.

PR-URL: #41488
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Merged
danielleadams pushed a commit that referenced this pull request Feb 26, 2022
@Trott @danielleadams
stream: avoid function call where possible
1425e75 
Instead of assigning a boolean, move the function call that assigns a
value to the boolean to the only place that boolean is checked. This
avoids the function call in cases where it is not needed.

Refs: #41488 (review)

PR-URL: #41534
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 left review comments

@mcollina mcollina mcollina approved these changes

@benjamingr benjamingr benjamingr approved these changes

@ronag ronag ronag approved these changes

@tniessen tniessen tniessen approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@Trott @nodejs-github-bot @mcollina @benjamingr @ronag @tniessen @aduh95 @RaisinTen @VoltrexKeyva