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

tools: refactor dynamic strings creation in shell scripts #45240

Merged
merged 2 commits into from Nov 1, 2022

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Oct 30, 2022

IMHO it's much easier to read the code when the whole string goes in one chunk, rather than having unnecessary concatenation – and that seems to work better with syntax highlighting as well.

E.g. if find

tar zxf "$WORKSPACE/cli/release/npm-$NPM_VERSION.tgz"
more readable than
tar zxf "$WORKSPACE"/cli/release/npm-"$NPM_VERSION".tgz

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Oct 30, 2022
tools/license-builder.sh Outdated Show resolved Hide resolved
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 30, 2022
@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Nov 1, 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 Nov 1, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/45240
FetchError: Invalid response body while trying to fetch https://api.github.com/graphql: Premature close
    at consumeBody (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:234:60)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Response.text (file:///opt/hostedtoolcache/node/18.12.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/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:51:18)
    at async Request.query (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:109:20)
    at async Request.queryAll (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:136:20)
    at async Request.gql (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:66:22)
    at async PRData.getComments (file:///opt/hostedtoolcache/node/18.12.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/3369325988

@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Nov 1, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 1, 2022
@nodejs-github-bot nodejs-github-bot merged commit acd6f6c into nodejs:main Nov 1, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in acd6f6c

RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022
PR-URL: #45240
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Nov 1, 2022
lucshi pushed a commit to lucshi/node that referenced this pull request Nov 9, 2022
PR-URL: nodejs#45240
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
PR-URL: #45240
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
targos pushed a commit that referenced this pull request Nov 24, 2022
PR-URL: #45240
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
PR-URL: #45240
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
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-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants