search

material-extensions / material-icons-browser-extension

Browser Addon that enhances file browsers of version controls with material icons.
MIT License
532 stars 41 forks source link

Consider a more lenient `engines` req in package.json #79

Closed AdamRaichu closed 3 months ago

AdamRaichu commented 8 months ago

As it is right now, package.json has the following values for the engines key.

https://github.com/Claudiohbsantos/github-material-icons-extension/blob/32fc7be79485b4c6fd4655700c380feb07114892/package.json#L16-L19

I tried to clone this repo and install by running npm i. I received an error telling me that my node and npm versions were not compatible. (I was using node v18.16.0 and npm 9.8.1.)

I believe you should consider changing the engine requirement from ^ (Include everything that does not increment the first non-zero portion of semver) to >= (Specify a range of stable versions).

https://semver.npmjs.com/

...Unless using a newer version of node would indeed break the repo, in which case I feel like you should mention that somewhere.

csandman commented 8 months ago

Haha, this is funny because I thought the same thing a while ago. Once upon a time (before I put up a PR to change it) it was using exact versions. I changed it to this to make it at least a bit more general, but that was a while ago and versions have gotten a lot newer. Tbh, there probably doesn't need to be an engines field at all. You could always just submit a PR to either remove it entirely or make it ">=16" and ">=8"?

PKief commented 3 months ago

We've removed the engine configuration for now. So you should be able to run the scripts with Node.js LTS (>=20). No need to have the exact node version on your machine.