src: move node_process to modern THROW_ERR* by jasnell · Pull Request #35472 · nodejs/node · GitHub

Semver-minor because it adds a code. Not semver-major because the error messages do not change. There is one changed detail tho: previously a couple of these functions would throw if too many arguments were passed in. I've relaxed that here given that it's not a check we consistently apply throughout.

Signed-off-by: James M Snell jasnell@gmail.com

Checklist

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

Signed-off-by: James M Snell <jasnell@gmail.com>

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

Semver-minor because it adds a code. Not semver-major because the error messages do not change.

Not going to fight it or anything, but isn't this backwards? Isn't the idea that message changes are semver-patch and changing (or adding) a code to an error is semver-major?

There is one changed detail tho: previously a couple of these functions would throw if too many arguments were passed in. I've relaxed that here given that it's not a check we consistently apply throughout.

Is there a use case for this and/or a record of it causing problems for folks? Or is the motive more about internal consistency?

Semver-minor because it adds a code. Not semver-major because the error messages do not change.

Not going to fight it or anything, but isn't this backwards? Isn't the idea that message changes are semver-patch and changing (or adding) a code to an error is semver-major?

Oh, wait, I see... semver-minor and not semver-patch, on the grounds that adding a code is minor but changing a code would be major. Sure. I would have thought major out of caution would be the default, but I also think the approach here makes sense so... ¯\(ツ)/¯

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

Commit Queue failed

- Loading data for nodejs/node/pull/35472
✔  Done loading data for nodejs/node/pull/35472
----------------------------------- PR info ------------------------------------
Title      src: move node_process to modern THROW_ERR* (#35472)
Author     James M Snell  (@jasnell)
Branch     jasnell:node_process_errors -> nodejs:master
Labels     C++, errors, lib / src, semver-minor
Commits    1
 - src: move node_process to modern THROW_ERR*
Committers 1
 - James M Snell 
PR-URL: https://github.com/nodejs/node/pull/35472
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/35472
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2020-10-04T00:50:42Z: https://ci.nodejs.org/job/node-test-pull-request/33359/
- Querying data for job/node-test-pull-request/33359/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Fri, 02 Oct 2020 21:52:07 GMT
   ✔  Approvals: 1
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/35472#pullrequestreview-501630565
   ✖  This PR needs to wait 100 more hours to land (or 0 hours if there is one more approval)
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #35472 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Landed in b28ba4b

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #35472 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Notable changes: crypto: * update certdata to NSS 3.56 (Shelley Vohr) #35546 deps: * update llhttp to 2.1.3 (Fedor Indutny) #35435 * (SEMVER-MINOR) upgrade to libuv 1.40.0 (Colin Ihrig) #35333 doc: * add aduh95 to collaborators (Antoine du Hamel) #35542 fs: * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134 http: * (SEMVER-MINOR) added scheduling option to http agent (delvedor) #33278 module: * (SEMVER-MINOR) exports pattern support (Guy Bedford) #34718 * (SEMVER-MINOR) named exports for CJS via static analysis (Guy Bedford) #35249 n-api: * (SEMVER-MINOR) add more property defaults (Gerhard Stoebich) #35214 src: * (SEMVER-MINOR) move node_contextify to modern THROW_ERR_* (James M Snell) #35470 * (SEMVER-MINOR) move node_process to modern THROW_ERR* (James M Snell) #35472 * (SEMVER-MINOR) expose v8::Isolate setup callbacks (Shelley Vohr) #35512 PR-URL: TODO

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #35472 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Notable changes: crypto: * update certdata to NSS 3.56 (Shelley Vohr) #35546 deps: * update llhttp to 2.1.3 (Fedor Indutny) #35435 * (SEMVER-MINOR) upgrade to libuv 1.40.0 (Colin Ihrig) #35333 doc: * add aduh95 to collaborators (Antoine du Hamel) #35542 fs: * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134 http: * (SEMVER-MINOR) added scheduling option to http agent (delvedor) #33278 module: * (SEMVER-MINOR) exports pattern support (Guy Bedford) #34718 * (SEMVER-MINOR) named exports for CJS via static analysis (Guy Bedford) #35249 n-api: * (SEMVER-MINOR) add more property defaults (Gerhard Stoebich) #35214 src: * (SEMVER-MINOR) move node_contextify to modern THROW_ERR_* (James M Snell) #35470 * (SEMVER-MINOR) move node_process to modern THROW_ERR* (James M Snell) #35472 * (SEMVER-MINOR) expose v8::Isolate setup callbacks (Shelley Vohr) #35512 PR-URL: #35950

src: move node_process to modern THROW_ERR*

941ee2e

Signed-off-by: James M Snell <jasnell@gmail.com>

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 2, 2020

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2020

joyeecheung approved these changes Oct 4, 2020

View reviewed changes

jasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 5, 2020

github-actions Bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 5, 2020

github-actions Bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 5, 2020

jasnell removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 5, 2020

jasnell added a commit that referenced this pull request Oct 10, 2020

src: move node_process to modern THROW_ERR*

b28ba4b

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #35472 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

jasnell closed this Oct 10, 2020

MylesBorins pushed a commit that referenced this pull request Oct 14, 2020

src: move node_process to modern THROW_ERR*

541082c

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #35472 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

MylesBorins mentioned this pull request Oct 14, 2020

v14.14.0 proposal #35648

Merged

MylesBorins pushed a commit that referenced this pull request Nov 3, 2020

src: move node_process to modern THROW_ERR*

dee6bc4

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #35472 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

MylesBorins mentioned this pull request Nov 3, 2020

v12.20.0 proposal #35950

Merged

MylesBorins pushed a commit that referenced this pull request Nov 16, 2020

src: move node_process to modern THROW_ERR*

b741f2f

Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #35472 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

This was referenced Jul 11, 2021

chore(deps): update node.js to >=v12.22.3 kufu/smarthr-ui#1741

Closed

chore(deps): update node.js -> >=v12.22.12 - engines - abandoned brody2consult/react-native-module-init#125

Open

gary-kim-bot mentioned this pull request Aug 2, 2021

chore(deps): update node.js to >=v12.22.4 gary-kim/nc-build-script#63

Open

1 task

renovate Bot mentioned this pull request Aug 3, 2021

Update Node.js to >=v12.22.4 Pytal/deploy-env#89

Merged

1 task

This was referenced Aug 11, 2021

Update Node.js to v12.22.5 SirReiva/backend-mongo-express-jwt#48

Closed

Update Node.js to v12.22.12 - abandoned shaimael-ws-java-demo/mariadb#41

Open

src: move node_process to modern THROW_ERR* by jasnell · Pull Request #35472 · nodejs/node · GitHub

Uh oh!

Uh oh!

src: move node_process to modern THROW_ERR* - #35472

src: move node_process to modern THROW_ERR*#35472
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:node_process_errors

jasnell commented Oct 2, 2020

Uh oh!

nodejs-github-bot commented Oct 2, 2020

Uh oh!

Trott commented Oct 3, 2020

Uh oh!

Trott commented Oct 3, 2020

Uh oh!

Trott commented Oct 3, 2020

Uh oh!

nodejs-github-bot commented Oct 4, 2020

Uh oh!

github-actions Bot commented Oct 5, 2020

Uh oh!

jasnell commented Oct 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

src: move node_process to modern THROW_ERR* by jasnell · Pull Request #35472 · nodejs/node · GitHub

Uh oh!

Uh oh!

src: move node_process to modern THROW_ERR* - #35472

src: move node_process to modern THROW_ERR*#35472jasnell wants to merge 1 commit intonodejs:masternodejs/node:masterfrom jasnell:node_process_errorsjasnell/node:node_process_errorsCopy head branch name to clipboard

Conversation

jasnell commented Oct 2, 2020

Checklist

Uh oh!

nodejs-github-bot commented Oct 2, 2020

Uh oh!

Trott commented Oct 3, 2020

Uh oh!

Trott commented Oct 3, 2020

Uh oh!

Trott commented Oct 3, 2020

Uh oh!

nodejs-github-bot commented Oct 4, 2020

Uh oh!

github-actions Bot commented Oct 5, 2020

Uh oh!

jasnell commented Oct 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

src: move node_process to modern THROW_ERR*#35472
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:node_process_errors