search

YoshiWalsh / docker-pterodactyl-panel

Easily set up Pterodactyl Panel on Docker, adhering to the *one service per container* mantra.
ISC License
36 stars 14 forks source link

Some improvements to the project #1

Closed noschang closed 3 years ago

noschang commented 3 years ago

Hi @JoshuaWalsh

I'm doing several modifications to the project in order to improve it and I think it would be nice to incorporate these changes in the original repository so other people can benefit from them. Feel free to contact me if you have any questions

YoshiWalsh commented 3 years ago

Hi @noschang, thanks for your interest and contributions.

I have some feedback regarding your changes:

  1. Each pull request should contain one self-contained change. This can be achieved via feature branches.
  2. .env files should not be committed to VCS. COMPOSE_PROJECT_NAME should be set by the person deploying the stack, not by us as project maintainers. I'd prefer to add instructions for setting this into the README.md rather than specify it in a .env

Thanks!

noschang commented 3 years ago

Hi @noschang, thanks for your interest and contributions.

I have some feedback regarding your changes:

1. Each pull request should contain one self-contained change. This can be achieved via feature branches.

2. `.env` files [should not be committed to VCS](https://github.com/motdotla/dotenv#should-i-commit-my-env-file). `COMPOSE_PROJECT_NAME` should be set by the person deploying the stack, not by us as project maintainers. I'd prefer to add instructions for setting this into the README.md rather than specify it in a `.env`

Thanks!

Hi @JoshuaWalsh. OK, I'll follow your recommendations. What about providing the file as sample.env and updating the README.md file instructing the user to rename it to .env and change the values to the desired ones?

YoshiWalsh commented 3 years ago

Hi @JoshuaWalsh. OK, I'll follow your recommendations. What about providing the file as sample.env and updating the README.md file instructing the user to rename it to .env and change the values to the desired ones?

I think best practice is just to instruct users to run COMPOSE_PROJECT_NAME=pterodactyl-panel docker-compose -d

noschang commented 3 years ago

I think best practice is just to instruct users to run COMPOSE_PROJECT_NAME=pterodactyl-panel docker-compose -d

@JoshuaWalsh OK. And what about the other variables?

For example, in my fork I moved the database user and password configuration to the sample.env file, because I believe it's more convenient to the user setting this once in the env file than having to change it in several places in the docker-compose.yml file. Also, the user can always override these variables in the command line.

Another reason for supplying a sample file is because I'm planning to add some more configuration variables.