Skip to content

lib: simplify several debug() calls - #25241

Merged
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:debug
Dec 29, 2018
Merged

lib: simplify several debug() calls#25241
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:debug

Conversation

@cjihrig

Copy link
Copy Markdown
Contributor

Avoid calling Array.prototype.join() in debug() calls. These are evaluated on every call, even if the debug() call is a no-op. This commit replaces the join() calls with the %j placeholder.

As a general note, we should probably migrate the codebase to using printf() style placeholders, and avoid template strings and string concatenation in debug() calls. It might be a good Code and Learn activity. The http2 module is a particularly bad offender.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-botnodejs-github-bot added dont-land-on-v6.x http2 Issues or PRs related to the http2 subsystem. labels Dec 27, 2018
@BridgeAR

Copy link
Copy Markdown
Member

we should probably migrate the codebase to using printf() style placeholders, and avoid template strings and string concatenation in debug() calls. It might be a good Code and Learn activity.

❤️ I am definitely 👍 on that.

@addaleax

Copy link
Copy Markdown
Member

I think template strings are a lot more JavaScript-y than printf-style replacement, so I would be against doing this in places where it's not causing runtime overhead.

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

@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 27, 2018
@cjihrig

Copy link
Copy Markdown
ContributorAuthor

Avoid calling Array.prototype.join() in debug() calls. These
are evaluated on every call, even if the debug() call is a
no-op. This commit replaces the join() calls with the %j
placeholder.
PR-URL: nodejs#25241
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig
cjihrig merged commit 69470c8 into nodejs:masterDec 29, 2018
@cjihrig
cjihrig deleted the debug branch December 29, 2018 18:53
targos pushed a commit that referenced this pull request Jan 1, 2019
Avoid calling Array.prototype.join() in debug() calls. These
are evaluated on every call, even if the debug() call is a
no-op. This commit replaces the join() calls with the %j
placeholder.
PR-URL: #25241
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
Avoid calling Array.prototype.join() in debug() calls. These
are evaluated on every call, even if the debug() call is a
no-op. This commit replaces the join() calls with the %j
placeholder.
PR-URL: nodejs#25241
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeARBridgeAR mentioned this pull request Jan 16, 2019
@MylesBorinsMylesBorins mentioned this pull request Jan 24, 2019
BethGriggs pushed a commit that referenced this pull request Apr 17, 2019
Avoid calling Array.prototype.join() in debug() calls. These
are evaluated on every call, even if the debug() call is a
no-op. This commit replaces the join() calls with the %j
placeholder.
PR-URL: #25241
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 28, 2019
Avoid calling Array.prototype.join() in debug() calls. These
are evaluated on every call, even if the debug() call is a
no-op. This commit replaces the join() calls with the %j
placeholder.
PR-URL: #25241
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggsBethGriggs mentioned this pull request May 1, 2019
BethGriggs pushed a commit that referenced this pull request May 10, 2019
Avoid calling Array.prototype.join() in debug() calls. These
are evaluated on every call, even if the debug() call is a
no-op. This commit replaces the join() calls with the %j
placeholder.
PR-URL: #25241
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
Avoid calling Array.prototype.join() in debug() calls. These
are evaluated on every call, even if the debug() call is a
no-op. This commit replaces the join() calls with the %j
placeholder.
PR-URL: #25241
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.http2Issues or PRs related to the http2 subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@cjihrig@nodejs-github-bot@BridgeAR@addaleax@jasnell@lpinca