search

OSC / ood-fileexplorer

[MOVED] The Open OnDemand File Explorer
https://osc.github.io/Open-OnDemand/
MIT License
4 stars 1 forks source link

Merge cloudcmd into this repo #175

Closed nickjer closed 5 years ago

nickjer commented 6 years ago

I believe it was discussed before, but can't remember the outcome of that discussion. If it is possible it would be beneficial to merge the https://github.com/OSC/cloudcmd/ project into this repo's code to reduce complexity of updating File Explorer.

ericfranz commented 6 years ago

The goal is to version the cloudcmd dependency code with the rest of the app, since this produces most of the client side code. This will make changes to the app that involve both app.js and the cloudcmd repo much easier, and will also let us experiment with replacing some controls that cloudcmd provides with some simpler options. Note that cloudcmd provides its own dependency list, so if we do this it might be required to add its dependencies to package.json.

Because the cloudcmd dependency is already in node_modules/cloudcmd it would be easier to just move this to a lib/cloudcmd directory and modify NODE_PATH environment variable. I wonder if you can actually set this at the very top of the app.js file. There are other options too:

  1. Set it in bin/node before you execute your app
  2. ~Set it in package.json a relative path to the dependency i.e. "./lib/cloudcmd" (it is available in npm 5 https://github.com/npm/npm/issues/15778)~ npm 3.10.9 is what is provided by software collections rh-nodejs6 so this won't work
  3. Set it in .env file and get rid of the deprecation warning. Move the loading of dotenv to the top of app.js.
  4. Set in package.json the start script which can be a command that sets NODE_PATH=./lib prior to executing app.js. I don't know if this works with Passenger.
ericfranz commented 6 years ago

So, it looks like yarn supports multiple package.json files: https://yarnpkg.com/en/docs/workspaces#toc-how-to-use-it

Its another approach that would let us use the cloudcmd dependency as a "workspace". I don't know if you can do subdirectory i.e. lib/cloudcmd/ or if you would just have in the root of the app cloudcmd/.

Note that yarn is not available with nodejs010 so you would have to wait till we updated to a later version of node.js to do this. And it might make the spec for installing slightly more complex, though bin/setup would just have to handle installing a local copy of yarn if it didn't exist.

MorganRodgers commented 5 years ago

This was completed in #180.