Skip to content

add an all plaform supported cleanup solution#780

Merged
ShogunPanda merged 7 commits into
nodejs:mainfrom
Vizonex:all-platform-support-rm
Mar 10, 2026
Merged

add an all plaform supported cleanup solution#780
ShogunPanda merged 7 commits into
nodejs:mainfrom
Vizonex:all-platform-support-rm

Conversation

@Vizonex

@Vizonex Vizonex commented Nov 6, 2025

Copy link
Copy Markdown
Contributor

There has been an ongoing issue for users using Windows who try and compile llhttp where the remove command rm wouldn't work (because it doesn't exist on windows) so I came up a logical solution that tries to fix this and hopefully make it so that other users don't fall under the same boat.

fixes #767

@ShogunPanda ShogunPanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks fine to me.
Just two minor things:

  1. Can you move it to the scripts folder?
  2. Can you make sure it's properly formatted according to our linting rules?

@Vizonex

Vizonex commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

I sure can, give me a couple of minutes.

@Vizonex

Vizonex commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

@ShogunPanda About what you told me I am in a bit of confusion as to what folder specifically to move this new clean.ts file.

  1. Can you move it to the scripts folder?

I had it in the bin folder due to believing it was where such scripts were to be stored but would I be moving this new clean.ts script to the src folder or does this request imply I make a new scripts folder for this specific tool?

@Vizonex Vizonex requested a review from ShogunPanda February 24, 2026 18:52
@ShogunPanda

Copy link
Copy Markdown
Contributor

@Vizonex My bad. I meant you to create a new /scripts folder.

@Vizonex

Vizonex commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Seems to be a little hiccup with linux. When using this script. I'm wondering if it's having problems with reaching the file or running it?

@ShogunPanda

Copy link
Copy Markdown
Contributor

@Vizonex Good question. Try with ls . or pass a relative path to the execution command line

@Vizonex

Vizonex commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

@ShogunPanda I fixed part of the script so that it resolves the items correctly but seems that linux is still not cooperating with me.

@ShogunPanda

Copy link
Copy Markdown
Contributor

Just add:

!scripts

at the end of .dockerignore and you should be good to go.

@Vizonex

Vizonex commented Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

@ShogunPanda Alright all I need now is the workflows to work properly.

@ShogunPanda ShogunPanda merged commit 85095fb into nodejs:main Mar 10, 2026
7 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot build parser on windows

2 participants