search

sequelize / umzug

Framework agnostic migration tool for Node.js
MIT License
1.98k stars 158 forks source link

bump glob to v10 #658

Closed mmkal closed 3 months ago

mmkal commented 3 months ago

closes #638

sort paths since they come back in reverse order now

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.45%. Comparing base (53c9a54) to head (754b225).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #658 +/- ## ========================================== - Coverage 99.45% 99.45% -0.01% ========================================== Files 12 12 Lines 1467 1465 -2 Branches 277 277 ========================================== - Hits 1459 1457 -2 Misses 8 8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

WikiRik commented 3 months ago

@mmkal Glob v10 does not support Node 12 as per the following; https://github.com/isaacs/node-glob/blob/b5d0f640893eba729bb8675a51a73ffbc0760b35/package.json#L96

Are you sure the tests on Node 12 passed?

mmkal commented 3 months ago

It looks like there's a bug that's making the Node 12 and Node 14 npm test commands pass even though they're both failing:

image

I will look into that, and maybe just switch to fast-glob which supports nodejs 8+.

B4nan commented 3 months ago

Why would you care about this? Ship a major version if the current one is supposed to work with this. Supporting node 16 is questionable (but understandable), supporting anything older is (IMO) just weird and a waste of (your/maintainer's) time.

Node 12 is not receiving security updates for 2 years now, node 14 for a year, and node 16 for more than 6 months.

WikiRik commented 3 months ago

Why would you care about this? Ship a major version if the current one is supposed to work with this.

If we are shipping this in a major version then it's not an issue and we can use glob v10 just fine. But as far as I know this was supposed to be released as a minor or patch version, which would break older projects and not be in the spirit of semantic versioning.

mmkal commented 3 months ago

I prefer not to release major versions unless there's a good reason. Especially for a library like this which is sometimes a couple layers deep in dependency trees. Using fast-glob is an easy fix, and not worth a major version IMO. The tests for lower node versions were just broken.

Having said that, eventually it will be too much of a headache to support old node versions. At that point I'm fine with a major bump.

github-actions[bot] commented 3 months ago

Released in v3.8.0.