inspector: open add `SymbolDispose` #48765

atlowChemi · 2023-07-14T07:43:18Z

No description provided.

benjamingr

I'm wondering if we can make the test simpler but LGTM

atlowChemi · 2023-07-14T09:14:42Z

I'm wondering if we can make the test simpler

I would be open to any suggestions 🙂
I based this on the existing .open() test, and did not think of anything better 😕

nodejs-github-bot · 2023-07-14T13:02:27Z

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

nodejs-github-bot · 2023-07-15T18:11:52Z

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

anonrig · 2023-07-16T01:07:55Z

Since we are changing a return type of a function, should we consider this change as a semver-major?

atlowChemi · 2023-07-16T02:52:55Z

Since we are changing a return type of a function, should we consider this change as a semver-major?

I have thought about it, and it might be preferable. 🤔
This changes from void, so I am not sure this is actually going to cause any issues (though people might be using this very differently than I'd expect) 🤷🏽‍♂️
I do not feel strongly either way - and if you think adding it is the right thing - I am happy to do so 🙂

MoLow

LGTM. I would probably consider this a semver-minor. CC @nodejs/releasers

github-actions · 2023-07-16T07:04:17Z

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @benjamingr.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment.

benjamingr · 2023-07-16T07:04:38Z

The inspector now supports the new upcoming Symbol.dispose symbol for resource management.

nodejs-github-bot · 2023-07-16T09:38:27Z

Commit Queue failed

- Loading data for nodejs/node/pull/48765 ✔ Done loading data for nodejs/node/pull/48765 ----------------------------------- PR info ------------------------------------ Title inspector: open add `SymbolDispose` (#48765) Author Chemi Atlow (@atlowChemi) Branch atlowChemi:inspector-open-add-dispose -> nodejs:main Labels semver-minor, inspector, notable-change, author ready, needs-ci Commits 2 - inspector: open add `SymbolDispose` - fix test Committers 1 - atlowChemi PR-URL: https://github.com/nodejs/node/pull/48765 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/48765 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 14 Jul 2023 07:43:18 GMT ✔ Approvals: 2 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/48765#pullrequestreview-1529845578 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/48765#pullrequestreview-1531671441 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-07-15T18:11:52Z: https://ci.nodejs.org/job/node-test-pull-request/52771/ - Querying data for job/node-test-pull-request/52771/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 48765 From https://github.com/nodejs/node * branch refs/pull/48765/merge -> FETCH_HEAD ✔ Fetched commits as a4e4a86d8920..1bdf897025cb -------------------------------------------------------------------------------- [main 254f8dd2b2] inspector: open add `SymbolDispose` Author: atlowChemi Date: Fri Jul 14 10:28:36 2023 0300 3 files changed, 88 insertions( ) create mode 100644 test/parallel/test-inspector-open-dispose.mjs [main f117a971e9] fix test Author: atlowChemi Date: Sat Jul 15 21:06:22 2023 0300 1 file changed, 3 insertions( ), 3 deletions(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
inspector: open add SymbolDispose

PR-URL: #48765
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Moshe Atlow [email protected]

[detached HEAD 489a9712bc] inspector: open add SymbolDispose
Author: atlowChemi [email protected]
Date: Fri Jul 14 10:28:36 2023 0300
3 files changed, 88 insertions( )
create mode 100644 test/parallel/test-inspector-open-dispose.mjs
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fix test

PR-URL: #48765
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Moshe Atlow [email protected]

[detached HEAD 5b312876fb] fix test
Author: atlowChemi [email protected]
Date: Sat Jul 15 21:06:22 2023 0300
1 file changed, 3 insertions( ), 3 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/5566971519

PR-URL: nodejs#48765 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>

PR-URL: #48765 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>

Notable changes: esm: * unflag import.meta.resolve (Guy Bedford) #49028 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: TBD

Notable changes: esm: * unflag import.meta.resolve (Guy Bedford) #49028 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185

Notable changes: esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185

Notable changes: doc: * add new TSC members (Michael Dawson) #48841 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185

Notable changes: doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 PR-URL: #49185

Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 PR-URL: #49185

deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185

Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185

Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) nodejs#48660 doc: * add new TSC members (Michael Dawson) nodejs#48841 * add rluvaton to collaborators (Raz Luvaton) nodejs#49215 esm: * unflag import.meta.resolve (Guy Bedford) nodejs#49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) nodejs#48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) nodejs#48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) nodejs#48765 module: * implement `register` utility (João Lenon) nodejs#46826 * make CJS load from ESM loader (Antoine du Hamel) nodejs#47999 src: * add built-in `.env` file support (Yagiz Nizipli) nodejs#48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) nodejs#48660 and nodejs#45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) nodejs#48975 PR-URL: nodejs#49185

nodejs-github-bot added inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. labels Jul 14, 2023

atlowChemi force-pushed the inspector-open-add-dispose branch from cb6d0d8 to 04f04e1 Compare July 14, 2023 07:45

atlowChemi marked this pull request as ready for review July 14, 2023 07:45

atlowChemi requested review from benjamingr and MoLow July 14, 2023 07:45

benjamingr approved these changes Jul 14, 2023

View reviewed changes

atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 14, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 14, 2023

atlowChemi added 2 commits July 15, 2023 21:06

inspector: open add SymbolDispose

b9e291f

fix test

1bdf897

atlowChemi force-pushed the inspector-open-add-dispose branch from 04f04e1 to 1bdf897 Compare July 15, 2023 18:06

atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 15, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 15, 2023

atlowChemi added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 15, 2023

MoLow approved these changes Jul 16, 2023

View reviewed changes

benjamingr added semver-minor PRs that contain new features and should be released in the next minor version. notable-change PRs with changes that should be highlighted in changelogs. labels Jul 16, 2023

atlowChemi added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 16, 2023

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 16, 2023

atlowChemi removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jul 16, 2023

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023

inspector: open add SymbolDispose

a26b26d

PR-URL: nodejs#48765 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023

inspector: open add SymbolDispose

a3b5909

PR-URL: nodejs#48765 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>

UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023

inspector: open add SymbolDispose

3e11c49

PR-URL: nodejs#48765 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>

RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023

inspector: open add SymbolDispose

67f9896

PR-URL: #48765 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>

UlisesGascon mentioned this pull request Aug 15, 2023

v20.6.0 proposal #49185

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

inspector: open add `SymbolDispose` #48765

inspector: open add `SymbolDispose` #48765

atlowChemi commented Jul 14, 2023

benjamingr left a comment

atlowChemi commented Jul 14, 2023

nodejs-github-bot commented Jul 14, 2023

nodejs-github-bot commented Jul 15, 2023

anonrig commented Jul 16, 2023

atlowChemi commented Jul 16, 2023

MoLow left a comment

github-actions bot commented Jul 16, 2023

benjamingr commented Jul 16, 2023

nodejs-github-bot commented Jul 16, 2023

PR-URL: #48765
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Moshe Atlow [email protected]

PR-URL: #48765
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Moshe Atlow [email protected]

Successfully rebased and updated refs/heads/main.

inspector: open add SymbolDispose #48765

inspector: open add SymbolDispose #48765

Conversation

atlowChemi commented Jul 14, 2023

benjamingr left a comment

Choose a reason for hiding this comment

atlowChemi commented Jul 14, 2023

nodejs-github-bot commented Jul 14, 2023

nodejs-github-bot commented Jul 15, 2023

anonrig commented Jul 16, 2023

atlowChemi commented Jul 16, 2023

MoLow left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 16, 2023

benjamingr commented Jul 16, 2023

nodejs-github-bot commented Jul 16, 2023

PR-URL: #48765 Reviewed-By: Benjamin Gruenbaum [email protected] Reviewed-By: Moshe Atlow [email protected]

PR-URL: #48765 Reviewed-By: Benjamin Gruenbaum [email protected] Reviewed-By: Moshe Atlow [email protected]

Successfully rebased and updated refs/heads/main.

inspector: open add `SymbolDispose` #48765

inspector: open add `SymbolDispose` #48765

PR-URL: #48765
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Moshe Atlow [email protected]

PR-URL: #48765
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Moshe Atlow [email protected]