Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to Dockerfile #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

MysticRyuujin
Copy link

@MysticRyuujin MysticRyuujin commented Apr 27, 2021

  • Use Alpine over Debian

  • Install NodeJS 14 - The current recommended version. The old script is depreciated.

  • Run useradd instead of adduser - Removes some error messages.

  • Switch from pm2 start script to pm2-runtime command. - This is the recommended way to run pm2 in docker. It also stops us from crashing when the app name changes and the log file is missing,

* Install NodeJS 14 - The current recommended version. The old script is depreciated.

* Run `useradd` instead of `adduser` - Removes some error messages.

* Switch from `pm2 start` script to `pm2-runtime` command. - This is the recommended way to run pm2 in docker. It also stops us from crashing when the app name changes and the log file is missing,
@MysticRyuujin
Copy link
Author

Heh, I didn't even see #3 - which is probably a better pull request in general.

In an effort to more align with goerli#3 goals I switched to node:14-alpine - but I'm not sure if modification of app.json to passing env variables to docker is more or less preferred so here's another option.
@ramonhoyo
Copy link

I was about to make a PR to fix the Dockerfile too, however It looks like nobody is active on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants