koala-framework / composer-extra-assets

Composer Plugin for installing Assets using native npm/bower
BSD 2-Clause "Simplified" License
42 stars 11 forks source link

Install nodejs locally if not found #4

Closed nsams closed 9 years ago

nsams commented 9 years ago

Integrate this package once it's stable:

https://github.com/thecodingmachine/nodejs-installer

moufmouf commented 9 years ago

Hi @nsams ,

Just opened a pull request (#5) to improve compatibility between nodejs-installer and composer-extra-assets. I was wondering: do you see anything that could be improved to "nodejs-installer" that could help? I'm considering tagging the current release as 1.0.0, but I'd like some feedback first. If you think there is any interesting missing feature, let me know! Otherwise, I'll tag the release.

nsams commented 9 years ago

Hm, interesting.

My thoughts:

Is modifying the $PATH a standard solution or more a hack? I don't have the best feeling with it.

What I had in mind: composer-extra-assets should require nodejs-installer and use the nodejs by using vendor/bin/node instead of node. That would not require modifying $PATH.

What is missing in nodejs-installer is that not only the root package minimumVersion etc. is fetched.

What do you think?

moufmouf commented 9 years ago

Hi @nsams ,

Is modifying the $PATH a standard solution or more a hack? I don't have the best feeling with it.

I understand your reluctance, it is indeed a hack. I made it an option includeBinInPath that is disabled by default.

What is missing in nodejs-installer is that not only the root package minimumVersion etc. is fetched.

Good news, I got rid of "minimumVersion". Now, there is only one option "version" that accepts constraints just like NodeJS (for instance: "~0.11"). Also, this option is fetched and merged from all packages, so any package can add a particular constraint on the NodeJS version and the plugin will try to find the best matching version from all the constraint of all the existing packages.

This time, I believe it's ready. Anything else I can improve? Shall I release 1.0?

nsams commented 9 years ago

Good news, I got rid of "minimumVersion". Now, there is only one option "version" that > accepts constraints just like NodeJS (for instance: "~0.11"). Also, this option is fetched > and merged from all packages, so any package can add a particular constraint on the NodeJS version and the plugin will try to find the best matching version from all the constraint of all the existing packages.

Cool, thanks.

I have created a PR for that integrates nodejs-installer: #6 Do you think that's the way to go?

This time, I believe it's ready. Anything else I can improve? Shall I release 1.0?

I didn't test on other platforms than linux yet, but other than that it's ready for release.

nsams commented 9 years ago

6 is merged