GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — typings: lib/internal/vm.js by GeoffreyBooth · Pull Request #50112 · nodejs/node

Conversation

@GeoffreyBooth

@GeoffreyBooth GeoffreyBooth commented Oct 10, 2023

Copy link
Copy Markdown
Member

Tiny PR to add a little JSDoc.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@GeoffreyBooth GeoffreyBooth added vm Issues and PRs related to the vm subsystem. typings Issues and PRs related to internal TypeScript declarations. labels Oct 10, 2023
@GeoffreyBooth
GeoffreyBooth requested a review from joyeecheung October 10, 2023 03:48
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 10, 2023
@GeoffreyBooth
GeoffreyBooth force-pushed the type-internal-compile-function branch from 1999445 to a280623 Compare October 10, 2023 04:09
anonrig
anonrig approved these changes Oct 10, 2023
View reviewed changes
@anonrig anonrig removed the needs-ci PRs that need a full CI run. label Oct 10, 2023
@GeoffreyBooth GeoffreyBooth added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Oct 10, 2023
joyeecheung
joyeecheung reviewed Oct 12, 2023
View reviewed changes
Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@aduh95

aduh95 commented Nov 29, 2023

Copy link
Copy Markdown
Contributor

This needs a rebase

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aduh95 aduh95 removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Nov 29, 2023
legendecas
legendecas reviewed Dec 11, 2023
View reviewed changes
Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@fhinkel

fhinkel commented Feb 3, 2024

Copy link
Copy Markdown
Contributor

@GeoffreyBooth thanks so much for opening a PR! To be honest, I don't think the doc annotation adds much in this case given they're just a long form of the variable/function names. Would it be OK if we close this without merging it? Thanks for understanding.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@fhinkel fhinkel closed this Feb 3, 2024
@GeoffreyBooth

GeoffreyBooth commented Feb 3, 2024

Copy link
Copy Markdown
Member Author

I don't think the doc annotation adds much in this case

The main thing it adds is hinting for the types of arguments in your editor as you work. This was waiting on another PR from @joyeecheung but it might be unblocked now. I'm not sure when I'll have time to rebase but anyone else can finish this.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth GeoffreyBooth reopened this Feb 3, 2024
@GeoffreyBooth
GeoffreyBooth force-pushed the type-internal-compile-function branch from a280623 to c9ca65c Compare February 3, 2024 18:49
@GeoffreyBooth
GeoffreyBooth requested review from joyeecheung and legendecas February 3, 2024 18:53
anonrig
anonrig approved these changes Feb 4, 2024
View reviewed changes
legendecas
legendecas reviewed Feb 4, 2024
View reviewed changes
Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth
GeoffreyBooth requested a review from legendecas February 4, 2024 17:59
legendecas
legendecas reviewed Feb 5, 2024
View reviewed changes

@legendecas legendecas left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM % one suggestion

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth
GeoffreyBooth requested a review from legendecas February 5, 2024 02:37
joyeecheung
joyeecheung reviewed Feb 5, 2024
View reviewed changes
Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth
GeoffreyBooth requested a review from joyeecheung February 5, 2024 16:33
legendecas
legendecas approved these changes Feb 5, 2024
View reviewed changes

@legendecas legendecas left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM if lint complaints are fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/vm.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Feb 5, 2024

Copy link
Copy Markdown
Member Author

LGTM if lint complaints are fixed.

Thanks, that looks like it fixed the lint check, if you don’t mind reapproving.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

fhinkel
fhinkel approved these changes Feb 5, 2024
View reviewed changes
@fhinkel

fhinkel commented Feb 5, 2024

Copy link
Copy Markdown
Contributor

I don't think the doc annotation adds much in this case

The main thing it adds is hinting for the types of arguments in your editor as you work. This was waiting on another PR from @joyeecheung but it might be unblocked now. I'm not sure when I'll have time to rebase but anyone else can finish this.

Gotcha! Thanks for following up. LGTM.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Feb 7, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 7, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/50112
✔  Done loading data for nodejs/node/pull/50112
----------------------------------- PR info ------------------------------------
Title      typings: lib/internal/vm.js (#50112)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     GeoffreyBooth:type-internal-compile-function -> nodejs:main
Labels     vm, author ready, typings, commit-queue-squash
Commits    7
 - typings: lib/internal/vm.js
 - @legendecas suggestion
 - lint
 - Apply suggestions from code review
 - Update lib/internal/vm.js
 - Apply suggestions from code review
 - Apply suggestions from code review
Committers 2
 - Geoffrey Booth 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/50112
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Chengzhong Wu 
Reviewed-By: Franziska Hinkelmann 
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50112
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Chengzhong Wu 
Reviewed-By: Franziska Hinkelmann 
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 10 Oct 2023 03:48:21 GMT
   ✔  Approvals: 4
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/50112#pullrequestreview-1861179506
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/50112#pullrequestreview-1864665265
   ✔  - Franziska Hinkelmann (@fhinkel): https://github.com/nodejs/node/pull/50112#pullrequestreview-1864005359
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/50112#pullrequestreview-1865292289
   ✔  Last GitHub CI successful
   ✘  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/7815976165
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@legendecas legendecas added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Feb 7, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Feb 14, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 14, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

GeoffreyBooth added 3 commits February 17, 2024 14:48
@GeoffreyBooth
typings: lib/internal/vm.js
c1797e0
@GeoffreyBooth
@legendecas suggestion
fb07f0c
@GeoffreyBooth
lint
2550c46
@GeoffreyBooth
GeoffreyBooth force-pushed the type-internal-compile-function branch from dfdf6a7 to 2550c46 Compare February 17, 2024 23:00
@nodejs-github-bot

nodejs-github-bot commented Feb 17, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Feb 17, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth GeoffreyBooth added the commit-queue PRs queued for automated landing through the Commit Queue. label Feb 18, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Feb 18, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 18, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/50112
✔  Done loading data for nodejs/node/pull/50112
----------------------------------- PR info ------------------------------------
Title      typings: lib/internal/vm.js (#50112)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     GeoffreyBooth:type-internal-compile-function -> nodejs:main
Labels     vm, author ready, typings, commit-queue-squash
Commits    3
 - typings: lib/internal/vm.js
 - @legendecas suggestion
 - lint
Committers 1
 - Geoffrey Booth 
PR-URL: https://github.com/nodejs/node/pull/50112
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Chengzhong Wu 
Reviewed-By: Franziska Hinkelmann 
Reviewed-By: Joyee Cheung 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50112
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Chengzhong Wu 
Reviewed-By: Franziska Hinkelmann 
Reviewed-By: Joyee Cheung 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - typings: lib/internal/vm.js
   ⚠  - @legendecas suggestion
   ⚠  - lint
   ℹ  This PR was created on Tue, 10 Oct 2023 03:48:21 GMT
   ✔  Approvals: 4
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/50112#pullrequestreview-1861179506
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/50112#pullrequestreview-1864665265
   ✔  - Franziska Hinkelmann (@fhinkel): https://github.com/nodejs/node/pull/50112#pullrequestreview-1864005359
   ✔  - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/50112#pullrequestreview-1865292289
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-02-17T23:55:37Z: https://ci.nodejs.org/job/node-test-pull-request/57165/
- Querying data for job/node-test-pull-request/57165/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/7949622474
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Feb 18, 2024

Copy link
Copy Markdown
Member Author

Can someone please reapprove this so it can land?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Trott
Trott approved these changes Feb 18, 2024
View reviewed changes
@GeoffreyBooth GeoffreyBooth added commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Feb 19, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Feb 19, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 0550bc1 into nodejs:main Feb 19, 2024
@nodejs-github-bot

nodejs-github-bot commented Feb 19, 2024

Copy link
Copy Markdown
Collaborator

Landed in 0550bc1

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos pushed a commit that referenced this pull request Feb 19, 2024
@GeoffreyBooth @targos
typings: lib/internal/vm.js
adb2c36
PR-URL: #50112
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
v21.7.0 proposal #51932
Merged
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@GeoffreyBooth @richardlau
typings: lib/internal/vm.js
a7a4cce
PR-URL: #50112
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
v20.12.0 proposal #52212
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes
@anonrig anonrig anonrig approved these changes
@joyeecheung joyeecheung joyeecheung approved these changes
@legendecas legendecas legendecas approved these changes
+1 more reviewer
@fhinkel fhinkel fhinkel approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. typings Issues and PRs related to internal TypeScript declarations. vm Issues and PRs related to the vm subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@GeoffreyBooth @aduh95 @fhinkel @nodejs-github-bot @Trott @anonrig @joyeecheung @legendecas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.