jakearchibald / wittr

Silly demo app for an online course
385 stars 555 forks source link

Update docker compose #24

Closed a-magdy closed 6 years ago

a-magdy commented 6 years ago
jakearchibald commented 6 years ago

Unless I'm misunderstanding, npm start will fail on windows.

s-slavchev commented 6 years ago

npm start test results:

a-magdy commented 6 years ago

Ooops, I didn't consider windows!

.. well I could remove the start script from package.json to avoid confusion, and leave start.sh to be executed in docker-compose, would that be an acceptable solution?

s-slavchev commented 6 years ago

@a-magdy I personally see no reason to have the start.sh in the project.

On the other hand I find the new docker-compose very useful. Tested on Ubuntu 16.04 LTS and works fine - witter service looks properly configured, the volume is properly mapped, the watcher is working just fine picking up the file changes...

My two cents

a-magdy commented 6 years ago

Thanks @s-slavchev for your comment :)

a-magdy commented 6 years ago

@s-slavchev @jakearchibald I updated the code to avoid any conflicts, and the changes so far are:

jakearchibald commented 6 years ago

@a-magdy thanks for this! Can you move the local-machine instructions above the docker ones? Generally I'd rather they tried that first. Cheers!

a-magdy commented 6 years ago

@jakearchibald Done :)

jakearchibald commented 6 years ago

Merged, thank you!