react-slingshot icon indicating copy to clipboard operation
react-slingshot copied to clipboard

Adding docker support

Open dockerizeallthethings opened this issue 6 years ago • 14 comments

  • Adding docker image files for both prod and dev use
  • Adding instructions to top level README.md to:
    • Run a copy of the prod version hosted on hub.docker to try out the project
    • Build and run an image that will listen to changes on the host machine (to allow developers to use an IDE on their host machine)

dockerizeallthethings avatar Dec 27 '17 02:12 dockerizeallthethings

Coverage Status

Coverage remained the same at 88.034% when pulling 94c264a3c43a8c424317dd3acee6557d2d797400 on dockerizeallthethings:adding-docker-support into 97fc20662fac2e6584b0fb3801d74d3b185b6cdb on coryhouse:master.

coveralls avatar Dec 27 '17 02:12 coveralls

@dockerizeallthethings, thanks for the PR! Could you open an issue and link it to this PR? In general, we like to have a discussion about what goes into the project and an issue is the perfect place to do that. Cheers.

nickytonline avatar Dec 30 '17 22:12 nickytonline

Thanks for the PR! Dumb question: How would we update the Docker image hosted on hub.docker later?

coryhouse avatar Dec 31 '17 03:12 coryhouse

If you have accounts on hub.docker I should be able to add them as contributors.

Also, you're certainly welcome to host your own version on hub.docker, if you'd prefer.

Lemme know if you have any other questions, and feel free to send me some hub.docker usernames to add :).

--RG

On Sat, Dec 30, 2017 at 7:46 PM, Cory House [email protected] wrote:

Thanks for the PR! Dumb question: How would we update the Docker image hosted on hub.docker later?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/coryhouse/react-slingshot/pull/522#issuecomment-354583571, or mute the thread https://github.com/notifications/unsubscribe-auth/Ago6YtvXGK21FEUl5djdE8H4Cob7IZ1Aks5tFwOigaJpZM4RNHOH .

dockerizeallthethings avatar Jan 01 '18 22:01 dockerizeallthethings

Great! I'm housecor

coryhouse avatar Jan 01 '18 22:01 coryhouse

Done!

I also created an issue linked to the PR, as per an request from a Nick Taylor (to which I can't seem to directly respond).

Happy New Year!

--RG

On Mon, Jan 1, 2018 at 2:51 PM, Cory House [email protected] wrote:

Great! I'm housecor

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/coryhouse/react-slingshot/pull/522#issuecomment-354682638, or mute the thread https://github.com/notifications/unsubscribe-auth/Ago6YmRCAcaVBWIwfuXZcHnT13lRe4Qqks5tGWFUgaJpZM4RNHOH .

dockerizeallthethings avatar Jan 01 '18 23:01 dockerizeallthethings

I don't know much about docker so I cannot approve or deny of these changes. I am definitely excited about them though.

kwelch avatar Jan 02 '18 18:01 kwelch

I actually do something very similar to this already for a few projects, it helps to alleviate the complexities with managing various versions of nodejs and npm packages on my mac. I mount the root of the project folder into the container so i can then run the build if i wanted.

mikedevita avatar Jan 03 '18 21:01 mikedevita

This is for implementing #529.

nickytonline avatar Feb 15 '18 02:02 nickytonline

I haven't tried this locally yet, will try to take a peek at this tomorrow. Just curious why alpine instead of slim for Node? I haven't done a whole lot of Docker yet. New job's immersing me though 😉

nickytonline avatar Feb 17 '18 15:02 nickytonline

I also don't know Docker well enough yet to vet this. I'm hoping to dive in soon for work but for now I'm no help on this.

coryhouse avatar Feb 18 '18 14:02 coryhouse

@dockerizeallthethings, sorry that this PR and issue has gone stale. Any ideas for the first point that @mikedevita mentions?

nickytonline avatar Jun 15 '18 04:06 nickytonline

I'm not a huge fan of this for a couple of reasons:

  • Only one Dockerfile should be provided and it should closely mimic a PROD environment.
  • The PROD Dockerfile should not build the project. This causes a lot of configuration headache. The dist folder should be built like normal, then copied over to the Docker image on creation.

If someone wants to develop using Docker, that should be put back on the developer, this PR is making A LOT of assumptions.

For example, I use react-slingshot in a multi-project build, but my react-slingshot project has dependencies on other projects that have to be built first. Therefore, the Dockerfile has to change accordingly. Also, the development Dockerfile assumes that we want to expose port 3000 and 3001, it could be that someone is using docker-compose and Nginx to proxy to 3000 and 3001.

In my opinion, this is a good start but not ready to merge. It's trying to accomplish too much. I'm going to FORK react-slingshot this weekend and try to implement some ENV configuration changes and I'll integrate Docker as well.

I'm going to attempt to solve :

  • https://github.com/coryhouse/react-slingshot/issues/589
  • Docker integration

acmitch avatar Oct 12 '18 22:10 acmitch

If anyone is curious about my Docker comment, check out the PR: https://github.com/coryhouse/react-slingshot/pull/590

acmitch avatar Oct 14 '18 16:10 acmitch