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

A handful of fixes/tweaks #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cyberops7
Copy link

This was a great starting point for me, but I had to make some changes to get things to work (plus a adding a few packages that are more quality of life improvements for troubleshooting).

Of note:

  • Dockerfile
    • The unescaped $'s in the sed commands breaks the sql.php file
    • Adding the PHP PDO for mysql to the image, since without this I was getting PHP errors trying to talk to the DB
  • default.conf
    • Fixing the misnamed mysql.php reference
    • Switching the order of index.php and index.html so that just going to your FQDN defaults to the index.html page and loads your BlueMap instance, instead of defaulting to the PHP index page.
  • Updating documentation to match the env's in the docker-compose and docker run examples, etc. to the Dockerfile sed environment variable names.

@divadsn
Copy link
Owner

divadsn commented Mar 27, 2024

Hey, thanks for your time and your PR!

I actually had planned to do some fixing as I was going to update my Minecraft server at home with the latest Bluemap plugin, but life keeps me busy 😅

Small note, in the Dockerfile, do we actually need those additional tools? I feel we should keep the image as slim as possible, I mean, it would be fine if we weren't using alpine as the base image 😜

@cyberops7
Copy link
Author

I'm just trying to pay it forward a bit since you saved me all the pain of getting php to work...I'd been chasing my tail for a while until I found your repo, so thank you!

3.20 looks to be the latest stable version. Version 4.x is not ready for prime time quite yet.

As for the extra packages in the Dockerfile, no, they certainly aren't needed for it to run. I just like to have basic tools like ping, nslookup, and vim in my images (even Alpine-based ones), but feel free to remove them from the PR. According to the PR settings, it looks like maintainers are allowed to make edits. If you'd prefer I push a new commit, I'm happy to do that too.

@divadsn
Copy link
Owner

divadsn commented Mar 27, 2024

I'll do it later then, I am heading now to sleep 😴

I would also look into using the docker-php-extensions install script, just in case the base image gets a newer php version in the future. Or pin it down to PHP 8.0 for a while if possible :)

@divadsn
Copy link
Owner

divadsn commented Apr 19, 2024

Sorry, I forgot to come back to this pull request, had no time after work.

Will try to prioritize this weekend and follow-up :)

@divadsn divadsn added the enhancement New feature or request label Apr 19, 2024
@divadsn divadsn self-assigned this Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants