-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
fix: restore node12 support (glob -> fast-glob) #660
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #660 +/- ##
=======================================
Coverage 99.45% 99.45%
=======================================
Files 12 12
Lines 1465 1466 +1
Branches 268 269 +1
=======================================
+ Hits 1457 1458 +1
Misses 8 8 ☔ View full report in Codecov by Sentry. |
CC @WikiRik any feedback on this change? |
I don't have enough insights in the usages of umzug and the globs people use. So I can't say much on this. It seems that there are well documented changes between the two libraries that people can refer to and the added benefit is that the indirect vulnerable dependency is resolved without dropping Node 12 support. |
Agreed. I am working on a change to CI which will highlight that node 12/node 14 support is broken currently. I'll merge that first, which should make CI go red, then confirm that this change fixes it. |
previous CI setup wasn't actually running, it was throwing an uncaught rejection and somehow exiting with code 0 anyway getting vitest to work on node 12 was miles away from working, so moving the node version tests to the `test_tgz` job, which is a simpler test that ensures the basics work. Also adds some small fixes to the library itself to get it _closer_ to working, but the `glob` dependency is the sticking point for node 12. 14, 16, 18 are now fine. `glob` dependency will be replaced in #660 If getting node 12 to work proves impossible, we might need to just go to a v4 release but hopefully not.
Released in v3.8.0. |
fixes #659
Note that there are some edge-case differences between glob and fast-glob, but it's very unlikely that users of umzug will hit them.
See glob docs for an in-depth comparison: https://github.com/isaacs/node-glob?tab=readme-ov-file#comparison-to-other-javascript-glob-implementations
If there's anyone that needs the old
glob
behaviour, they can use glob themselves and pass an array of migrations to umzug. (see https://github.com/sequelize/umzug#direct-migrations-list)