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.

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

chore: switch to tinyglobby #3133

Merged
merged 1 commit into from
Feb 19, 2025
Merged

chore: switch to tinyglobby #3133

merged 1 commit into from
Feb 19, 2025

Conversation

benmccann
Copy link
Contributor

Checklist
  • npm install && npm run lint && npm test passes
  • tests are included
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

https://npmgraph.js.org/?q=glob - 26 dependencies
https://npmgraph.js.org/?q=tinyglobby - 2 dependencies

@cclauss
Copy link
Contributor

cclauss commented Feb 19, 2025

I believe this pull request should be made on https://github.com/nodejs/gyp-next. Once it is merged there, it will be vendored into this repo.

Perhaps I am wrong about this. @lukekarrys

@benmccann
Copy link
Contributor Author

Nice to see you both here!

I don't see any .js files in the gyp-next repo

@cclauss
Copy link
Contributor

cclauss commented Feb 19, 2025

OK... Could any tests that could be added to this pull request to verify that tinyglobby does what the old module did?

@lukekarrys
Copy link
Member

lukekarrys commented Feb 19, 2025

Our usage of globbing in the build.js file is very simple. Two async calls for files in the build/ dir with a specific extenion, and without no other options. I'm pretty confident that existing tests would cover this, and I'm comfortable landing as is.

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Thanks for doing this.

@lukekarrys
Copy link
Member

Thanks @benmccann!

@lukekarrys lukekarrys merged commit c3b3ab0 into nodejs:main Feb 19, 2025
46 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.

3 participants