Backport few changes previously done in RHOAR container #226
Conversation
Removing ln -s /usr/lib/node_modules/nodemon/bin/nodemon.js /usr/bin/nodemon from Dockerfiles because it does not work anyway for SCLs. Moving it in to nodejs_container_install script that can be used for more actions. Also fix path for /usr/bin/nodemon symlink, the previous did not work for SCLs. Similar to RHOAR image, DEBUG_PORT is specified directly in the Dockerfile. Other steps included in nodejs_container_install file are originally in the install_node.sh file in the RHOAR image.
Most notable changes: * npx is used for nodemon execution * GIT_COMMITTER_NAME and GIT_COMMITTER_EMAIL variables are set * npm cache is cleared by rm -rf .npm * npm install is run once more when starting the node * run npm run build when a build script is present * run yarn if is set, although we do not ship yarn in our images
We've actually had a test for specifically this, that cache is not removed. It was added by this commit: sclorg@0e9b5cb So it is probable better to not removing it.
When only one package is installed, it says added 1 package. Also print the overall test result after the cleanup messages, so it is more visible.
|
[test-openshift] |
There was a problem hiding this comment.
I am missing any documentation about DEBUG_PORT. Like here https://github.com/sclorg/s2i-nodejs-container/tree/master/10#environment-variables and https://github.com/sclorg/s2i-nodejs-container/tree/master/12#environment-variables
| echo -e "Current git config" | ||
| git config --list | ||
|
|
||
| if [ -n "$YARN_ENABLED" ]; then |
There was a problem hiding this comment.
Implementation of YARN_ENABLED is fine. But we will have the same problem with NEXUS_REGISTRY.
I would prefer to add to the condition NPM_MIRROR also yarn config set registry or define a new variable called YARN_MIRROR.
| echo -e "Current git config" | ||
| git config --list | ||
|
|
||
| if [ -n "$YARN_ENABLED" ]; then |
There was a problem hiding this comment.
The same here with YARN registry
|
Thanks for the PR! Nicely done with a couple of comments. |
|
@hhorak Is this still valid? If so, I would prefer to update it. If not feel free to close it. |
Most notable changes:
As a site-effect, these changes were done:
More details in the commit messages in particular commits.