Skip to content

Conversation

@christopher-besch
Copy link

@christopher-besch christopher-besch commented Oct 19, 2023

Hello smart people,

I created a Docker image for this project.
Additionally I added a full example deployment using Docker Compose so people can jump right into testing MinecraftStats.

Since I really like working with Docker environments and wanted to use the MinecraftStats project with my Minecraft server running inside Docker I added support for it.
I also wrote Documentation and a troubleshooting step for a problem I was stuck at for a while.

For the time being I published the image under my name as chrisbesch/minecraft_stats.
I'd like you, @pdinklag, to build the image yourself and publish it under your name instead since it's still your and not my project.

I'm looking forward to your comments!

@christopher-besch christopher-besch marked this pull request as ready for review October 19, 2023 07:38
@pdinklag
Copy link
Owner

Hey!
Thanks for providing this! A Docker container has been an open issue for a while, but since #182 never got any updates and I absolutely never (really) used Docker to be able to tell what's good what isn't, it's nice to have a PR that I can simply checkout and try. I'll do that in the coming week and put it in a separate branch first.

I also wrote Documentation and a troubleshooting step for a problem I was stuck at for a while.

That's the really important part here I will be having a look at!

Regarding the file structure, as I mentioned, I am not really versed with Docker and probably the Dockerfile should remain in the repository root, but I believe everything else shouldn't, including the entry point. Since it's just a shell script and the paths seem fixed to /app/... anyway, I guess it won't hurt putting everything pertaining to Docker in a docker/ directory. That'd be my only change request as far as I can tell now. Everything else can be fixed by contributors.

For the time being I published the image under my name as chrisbesch/minecraft_stats.
I'd like you, @pdinklag, to build the image yourself and publish it under your name instead since it's still your and not my project.

That would mean I'd have to maintain it under updates, and my activity on this project has already become pretty sparse due to limited time, so it may actually be better if somebody who actively uses Docker also maintains a Docker image. I will happily link to your image in the readme.

@christopher-besch
Copy link
Author

Thanks a lot for the positive reply, Patrick!
If you have any questions about deploying with Docker for the first time and/or can't make sense of my documentation feel free to reach out. I'm happy to help and do agree that documentation is key!

And yes, having the Dockerfile in the root is the standard everyone uses. I don't think it's a good idea to change that. But if you like I could put the entrypoint.sh and example Docker Compose deployment in a separate docker directory. It would make clear that the entrypoint.sh is not needed for a deployment on the host.

Personally I'm fine with maintaining the Docker image. I could watch this Repo and make sure that when things change, the Docker deployment still works. And you could always reach out to me when there are any Docker related issues.

@christopher-besch
Copy link
Author

@pdinklag, what do you say, can this be merged?

@christopher-besch
Copy link
Author

Did you get to try the Docker installation yet? I have added support for custom stats using Docker. So now the Docker install should allow everything the normal installation can do.

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