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 — bootstrap: simplify initialization of source map handlers by joyeecheung · Pull Request #48304 · nodejs/node

Conversation

@joyeecheung

@joyeecheung joyeecheung commented Jun 2, 2023

Copy link
Copy Markdown
Member
  • Move the initialization of process.setSourceMapsEnabled and the maybeCacheGeneratedSourceMap callback to bootstrap/node.js so they are included in the snapshot.
  • Simplify the handling of --enable-source-maps by explicitly calling setSourceMapsEnabled() during pre-execution.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@joyeecheung
bootstrap: simplify initialization of source map handlers
d6d09ea 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.
@nodejs-github-bot

nodejs-github-bot commented Jun 2, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup
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 lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jun 2, 2023
anonrig
anonrig approved these changes Jun 2, 2023
View reviewed changes
VoltrexKeyva
VoltrexKeyva approved these changes Jun 2, 2023
View reviewed changes
Comment thread lib/internal/source_map/source_map_cache.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
jasnell
jasnell approved these changes Jun 3, 2023
View reviewed changes

@jasnell jasnell 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 with the linting issues fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
legendecas
legendecas approved these changes Jun 6, 2023
View reviewed changes
@joyeecheung @VoltrexKeyva
fixup! bootstrap: simplify initialization of source map handlers
9495529 
Co-authored-by: Mohammed Keyvanzadeh <[email protected]>
@nodejs-github-bot

nodejs-github-bot commented Jun 6, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52125/
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 Jun 7, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52141/
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 Jun 8, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52158/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@joyeecheung joyeecheung added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Jun 8, 2023
@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 Jun 8, 2023
@nodejs-github-bot

nodejs-github-bot commented Jun 8, 2023

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/48304
✔  Done loading data for nodejs/node/pull/48304
----------------------------------- PR info ------------------------------------
Title      bootstrap: simplify initialization of source map handlers (#48304)
Author     Joyee Cheung  (@joyeecheung)
Branch     joyeecheung:source-map-init -> nodejs:main
Labels     process, lib / src, needs-ci, commit-queue-squash
Commits    2
 - bootstrap: simplify initialization of source map handlers
 - fixup! bootstrap: simplify initialization of source map handlers
Committers 2
 - Joyee Cheung 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/48304
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Mohammed Keyvanzadeh 
Reviewed-By: James M Snell 
Reviewed-By: Chengzhong Wu 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/48304
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Mohammed Keyvanzadeh 
Reviewed-By: James M Snell 
Reviewed-By: Chengzhong Wu 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - fixup! bootstrap: simplify initialization of source map handlers
   ℹ  This PR was created on Fri, 02 Jun 2023 17:53:13 GMT
   ✔  Approvals: 4
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/48304#pullrequestreview-1458168465
   ✔  - Mohammed Keyvanzadeh (@VoltrexKeyva): https://github.com/nodejs/node/pull/48304#pullrequestreview-1458238733
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/48304#pullrequestreview-1459828667
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/48304#pullrequestreview-1464502401
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-06-08T08:43:09Z: https://ci.nodejs.org/job/node-test-pull-request/52158/
- Querying data for job/node-test-pull-request/52158/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/5216112521
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
joyeecheung added a commit that referenced this pull request Jun 9, 2023
@joyeecheung
bootstrap: simplify initialization of source map handlers
4ff55ec 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.

PR-URL: #48304
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@joyeecheung

joyeecheung commented Jun 9, 2023

Copy link
Copy Markdown
Member Author

Landed in 4ff55ec
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@joyeecheung joyeecheung closed this Jun 9, 2023
This was referenced Jun 11, 2023
CI Reliability 2023-06-11 nodejs/reliability#590
Open
CI Reliability 2023-06-12 nodejs/reliability#591
Open
CI Reliability 2023-06-13 nodejs/reliability#592
Open
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
@joyeecheung @RafaelGSS
bootstrap: simplify initialization of source map handlers
4066295 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.

PR-URL: #48304
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jul 3, 2023
v20.4.0 proposal #48643
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@joyeecheung @Ceres6
bootstrap: simplify initialization of source map handlers
b6c9295 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.

PR-URL: nodejs#48304
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@joyeecheung @Ceres6
bootstrap: simplify initialization of source map handlers
c860d01 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.

PR-URL: nodejs#48304
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@ruyadorno

ruyadorno commented Aug 29, 2023

Copy link
Copy Markdown
Member

hi @joyeecheung these commits are not landing cleanly on v18.x-staging and will need manual backport in case we want to land this in v18.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ruyadorno ruyadorno added the backport-requested-v18.x label Aug 29, 2023
targos pushed a commit that referenced this pull request Nov 10, 2023
@joyeecheung @targos
bootstrap: simplify initialization of source map handlers
cf9ddcd 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.

PR-URL: #48304
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@targos targos mentioned this pull request Nov 28, 2023
v18.19.0 release proposal #50953
Merged
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
bootstrap: simplify initialization of source map handlers
3734b95 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.

PR-URL: nodejs/node#48304
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
bootstrap: simplify initialization of source map handlers
7cde3de 
- Move the initialization of process.setSourceMapsEnabled
  and the maybeCacheGeneratedSourceMap callback to
  bootstrap/node.js so they are included in the snapshot.
- Simplify the handling of --enable-source-maps by explicitly
  calling setSourceMapsEnabled() during pre-execution.

PR-URL: nodejs/node#48304
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@anonrig anonrig anonrig approved these changes
@legendecas legendecas legendecas approved these changes
+1 more reviewer
@VoltrexKeyva VoltrexKeyva VoltrexKeyva 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

commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. commit-queue-squash PRs the Commit Queue should land as one squashed commit. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. process Issues and PRs related to the process 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.

7 participants

@joyeecheung @nodejs-github-bot @ruyadorno @jasnell @anonrig @legendecas @VoltrexKeyva
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.