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.

Sign up for GitHub

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

Jump to bottom

[v22.x backport] module: add module.stripTypeScriptTypes #56208

Closed
wants to merge 3 commits into from
Closed

[v22.x backport] module: add module.stripTypeScriptTypes #56208

wants to merge 3 commits into from

Conversation

marco-ippolito
Copy link
Member

@marco-ippolito marco-ippolito commented Dec 10, 2024 •
edited
Loading

These commits are all related and sequential

@marco-ippolito marco-ippolito marked this pull request as ready for review December 10, 2024 14:58
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v22.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Dec 10, 2024
@marco-ippolito marco-ippolito added request-ci Add this label to start a Jenkins CI on a PR. module Issues and PRs related to the module subsystem. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Dec 10, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Dec 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

Failed to start CI
   ⚠  No approving reviews found
   ✘  Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/12258957548

@aduh95 aduh95 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Dec 10, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 10, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@aduh95
Copy link
Contributor

aduh95 commented Dec 10, 2024

Can you please rebase?

@marco-ippolito marco-ippolito force-pushed the backport-strip-typescript-types branch from 5b60e37 to 27465d9 Compare December 11, 2024 10:47
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 11, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 11, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@marco-ippolito marco-ippolito force-pushed the backport-strip-typescript-types branch from 27465d9 to fb9416c Compare December 13, 2024 18:22
@marco-ippolito
Copy link
Member Author

Linting failures are unrelated to the PR...

@aduh95 aduh95 requested a review from a team as a code owner December 13, 2024 19:48
@aduh95
Copy link
Contributor

aduh95 commented Dec 13, 2024

I've fixed the linter, can you please rebase?

@marco-ippolito marco-ippolito force-pushed the backport-strip-typescript-types branch from fb9416c to 2756f71 Compare December 14, 2024 07:23
@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@marco-ippolito
Copy link
Member Author

@nodejs/build I get this weird failure on windows 🤔
Screenshot 2024-12-16 at 14 08 40

@targos
Copy link
Member

targos commented Dec 16, 2024

There isn't this error in the current build (still running)

@richardlau
Copy link
Member

@nodejs/build I get this weird failure on windows 🤔 Screenshot 2024-12-16 at 14 08 40

I think it's due to e03be6b -- this was reverted on main by #56151 and the build machines updated to the newer Visual Studio version: nodejs/build#3963 (comment)

@richardlau
Copy link
Member

There isn't this error in the current build (still running)

I'm really confused by https://ci.nodejs.org/job/node-test-pull-request/64067/ -- it seems to link to https://ci.nodejs.org/job/node-test-commit/76453/ but that is for #56271.

https://ci.nodejs.org/job/node-test-pull-request/64067/console shows

06:53:48 Starting build job node-test-commit at 01:53:48.
06:53:48 Counting node-test-commit. enabledIndex=1
10:46:47 node-test-commit: Quiet period groovy=[index < 5 ? 0 : 2 * 60], index=1 -> quietPeriodGroovy=0
10:46:47 quiet period for node-test-commit is 0 seconds.Finished Build : #76448 of Job : node-test-commit with status : FAILURE at 05:46:47
10:46:47 Starting to gather test results!

and https://ci.nodejs.org/job/node-test-commit/76448/ does show the failure.

@marco-ippolito marco-ippolito force-pushed the backport-strip-typescript-types branch from 2756f71 to 8451932 Compare December 18, 2024 12:59
@marco-ippolito
module: simplify ts under node_modules check
ec6a002 
PR-URL: nodejs#55440
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@marco-ippolito
module: add module.stripTypeScriptTypes
2ee876c 
PR-URL: nodejs#55282
Fixes: nodejs#54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@marco-ippolito
doc: move typescript support to active development
587f3e8 
PR-URL: nodejs#55536
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@marco-ippolito marco-ippolito force-pushed the backport-strip-typescript-types branch from 8451932 to 587f3e8 Compare December 18, 2024 13:00
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 18, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 18, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@marco-ippolito

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 19, 2024
ruyadorno pushed a commit that referenced this pull request Dec 20, 2024
@marco-ippolito @ruyadorno
module: simplify ts under node_modules check
420d599 
PR-URL: #55440
Backport-PR-URL: #56208
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ruyadorno pushed a commit that referenced this pull request Dec 20, 2024
@marco-ippolito @ruyadorno
module: add module.stripTypeScriptTypes
7ed5aad 
PR-URL: #55282
Backport-PR-URL: #56208
Fixes: #54300
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
ruyadorno pushed a commit that referenced this pull request Dec 20, 2024
@marco-ippolito @ruyadorno
doc: move typescript support to active development
016d609 
PR-URL: #55536
Backport-PR-URL: #56208
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@ruyadorno
Copy link
Member

Landed in 5d92a5f...016d609

@ruyadorno ruyadorno closed this Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@JakobJingleheimer JakobJingleheimer JakobJingleheimer approved these changes

@ruyadorno ruyadorno ruyadorno approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. v22.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@marco-ippolito @nodejs-github-bot @aduh95 @targos @richardlau @ruyadorno @JakobJingleheimer