blocktronics / moebius

Modern ANSI & ASCII Art Editor
https://blocktronics.github.io/moebius/
Apache License 2.0
756 stars 41 forks source link

Adds Dockerfile + docker-compose.yml #144

Closed lvm closed 4 years ago

lvm commented 4 years ago

Hey there, glad to know you de-archived the repo.

Took the liberty to add a Dockerfile + docker-compoose, mainly to start a server quickly. I think this could be beneficial.
On the other hand, I merged a PR from another user that applied a bunch of changes to the README, if you think this adds an innecessary overhead to the PR, let me know and i'll see to create a new one without these changes.

andyherbert commented 4 years ago

The README is already fine as-is. I wouldn't mind some short mention of docker in there before I add any docker-related cruft.

andyherbert commented 4 years ago

To be honest I said "docker-related cruft" because I'm fairly ignorant about docker, I know what it does but I've never had to use it. In case I wanted to start a permanent Moebius server in the past I've paid for a cheap vm at somewhere like Linode, and I assume this will also be the case for everyone else.

I essentially agree with bart-d that if it's to be included then really it should be configured with the most obvious options and in a general purpose way. I was under the impression that @christianvozar already added a docker file? I kind of ignored what was already there because it always appeared that it wasn't particularly difficult to setup anyway, and I was tempted to remove all references to docker anyway because it essentially gives two ways to run a server, and someone who isn't particularly au-fait with the command line would probably get confused as a result.

lvm commented 4 years ago

Well, in that case a less pedantic review would've been a bit more friendly, i suppose.

No, there was no dockerfile at sight, otherwise i wouldn't ve bothered :D, and it isn't about how hard to setup it is but how convenient that this is already provided, because i can always open a notepad to draw ascii yet i'm here using a Node app to write text :-)

the docker run command doesn't enforce any argument, just presents an example of how you can run the server using docker.

Sorry to bother with this PR.

andyherbert commented 4 years ago

I don't think anyone is being pedantic, to be fair, I always appreciate that people would like to contribute but I think for stuff like this it would be better to raise an issue and say "would this be useful?" before putting the work in to avoid frustration.

That being said if you have a single line fix for a bug, or even better, a single line fix for a bug I don't know about then you can forgo the asking part.

lvm commented 4 years ago

I don't think anyone is being pedantic, to be fair,

Instead of giving a proper review or "yeah, but we don't like docker", i fear it falls in the pedantic category :-)

I always appreciate that people would like to contribute but I think for stuff like this it would be better to raise an issue and say "would this be useful?" before putting the work in to avoid frustration.

That being said if you have a single line fix for a bug, or even better, a single line fix for a bug I don't know about then you can forgo the asking part.

No worries, it took more of my time to deal with this thread than to create the dockerfile.

andyherbert commented 4 years ago

I guess I was kind of remiss for not checking the changes before commenting, but I was using my phone to buy essentials at the local supermarket and I had a few minutes spare to compose a response.

lvm commented 4 years ago

@andyherbert really, it's all good.

christianvozar commented 4 years ago

I have a branch with a Dockerfile. I modified the server.js to be more 12f app friendly (use ENV as well as flags, etc.). I use Docker with Moebius to setup multiple Moebius servers on one instance. With some Docker port mapping, proper CNAMEs and load balancing to the respective container this is easy to do and saves on additional hardware. I don’t expect anyone other than me to run Moebius in an image so I have not felt the need to commit it up.