Skip to content

util: remove unicode support todo for perf implications#43762

Merged
aduh95 merged 2 commits into
nodejs:mainfrom
Anemy:util-update-format-string-length-remove-todo-for-perf
Jul 24, 2022
Merged

util: remove unicode support todo for perf implications#43762
aduh95 merged 2 commits into
nodejs:mainfrom
Anemy:util-update-format-string-length-remove-todo-for-perf

Conversation

@Anemy
Copy link
Copy Markdown
Contributor

@Anemy Anemy commented Jul 10, 2022

Follow up from benchmarks in pr:
#43721

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Jul 10, 2022
@Anemy Anemy mentioned this pull request Jul 10, 2022
Closed
lpinca
lpinca reviewed Jul 10, 2022
View reviewed changes
Comment thread lib/internal/util/inspect.js
joyeecheung
joyeecheung approved these changes Jul 11, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

LGTM with Ipinca's suggestion

@BridgeAR BridgeAR removed the needs-ci PRs that need a full CI run. label Jul 11, 2022
@F3n67u F3n67u added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 16, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 20, 2022
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Jul 20, 2022

Commit Queue failed
- Loading data for nodejs/node/pull/43762
FetchError: Invalid response body while trying to fetch https://api.github.com/graphql: Premature close
    at consumeBody (file:///opt/hostedtoolcache/node/16.16.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:234:60)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Response.text (file:///opt/hostedtoolcache/node/16.16.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:158:18)
    at async Request.json (file:///opt/hostedtoolcache/node/16.16.0/x64/lib/node_modules/node-core-utils/lib/request.js:51:18)
    at async Request.query (file:///opt/hostedtoolcache/node/16.16.0/x64/lib/node_modules/node-core-utils/lib/request.js:109:20)
    at async Request.queryAll (file:///opt/hostedtoolcache/node/16.16.0/x64/lib/node_modules/node-core-utils/lib/request.js:136:20)
    at async Request.gql (file:///opt/hostedtoolcache/node/16.16.0/x64/lib/node_modules/node-core-utils/lib/request.js:66:22)
    at async PRData.getComments (file:///opt/hostedtoolcache/node/16.16.0/x64/lib/node_modules/node-core-utils/lib/pr_data.js:97:21)
    at async Promise.all (index 2)
    at async Promise.all (index 1) {
  type: 'system',
  errno: 'ERR_STREAM_PREMATURE_CLOSE',
  code: 'ERR_STREAM_PREMATURE_CLOSE',
  erroredSysCall: undefined
}
https://github.com/nodejs/node/actions/runs/2704963528

@aduh95 aduh95 merged commit 294ccf3 into nodejs:main Jul 24, 2022
@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented Jul 24, 2022

Landed in 294ccf3

danielleadams pushed a commit that referenced this pull request Jul 26, 2022
@Anemy @danielleadams
util: remove unicode support todo for perf implications
1af7f24 
PR-URL: #43762
Refs: #43721
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@danielleadams danielleadams mentioned this pull request Jul 26, 2022
Merged
targos pushed a commit that referenced this pull request Jul 31, 2022
@Anemy @targos
util: remove unicode support todo for perf implications
6dfbe12 
PR-URL: #43762
Refs: #43721
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Jul 31, 2022
@Anemy @targos
util: remove unicode support todo for perf implications
aff813f 
PR-URL: #43762
Refs: #43721
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Aug 1, 2022
@Anemy @targos
util: remove unicode support todo for perf implications
24cabbf 
PR-URL: #43762
Refs: #43721
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@targos targos mentioned this pull request Aug 3, 2022
Merged
viceice-bot added a commit to renovate-reproductions/docker-node-versioning-issue that referenced this pull request Aug 10, 2022
@viceice-bot