tdmalone / slackemon

Inspired by Pokémon Go, now you can catch and battle Pokémon with your teammates on Slack!
GNU General Public License v3.0
10 stars 3 forks source link

Remove sample config, reads env var by default #5

Closed Naramsim closed 7 years ago

Naramsim commented 7 years ago

Please, look at #4 before, if you want to merge, merge after #4. This PR contains your config.php removed from .gitignore, and deletes the sample one.

Inside config.php I assign variables looking from env and Dotenv.

One thing: define( 'SLACK_KEY__TXXXXXXXX', 'xoxp-0000000000-000000000000-000000000000-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' ); You are defining a key "SLACK_KEY__TXXXXXXXX", why all those Xs?

tdmalone commented 7 years ago

Thanks, I will review! The TXXXXXXXX is the team ID, so that one install of Slackemon can be configured to run across multiple teams. Each team will have their own app token and OAuth key generated by Slack.

juz501 commented 7 years ago

phpdotenv is not supposed to be used in a production environment according to the author vlucas https://github.com/vlucas/phpdotenv/issues/63#issuecomment-89419664

A proposed solution is to only load from .env in development only as below

Copying comment from #4

$dotenv = new Dotenv\Dotenv();
if(getenv('APP_ENV') === 'development') {
    $dotenv->load(__DIR__);
}
tdmalone commented 7 years ago

Ah, just had a look at config.php. To reduce complexity at this stage, I'm going to not try to implement multiple teams at the moment. I would like to in future, but I need to focus on understanding the best way to structure the environment first - with one team.

tdmalone commented 7 years ago

@Naramsim @juz501 This has been merged into tim and I've done further work on it too. I think I've got the config structure right now but value your further input. I need to test it further with Docker locally, and then ensure that the correct config vars are used through the app. Then I'll get Docker support into master.

tdmalone commented 7 years ago

Actually I can't build at the moment - Docker is giving me 'unknown instruction: USERMOD'. Will look into further tomorrow.

juz501 commented 7 years ago

I don't think you can add the \ to end the line and then append some comments after that (in the Dockerfile). I'm just guessing though.

You can just get rid of the && \ and just put RUN per line in the Dockerfile and try.

Naramsim commented 7 years ago

@juz501, Hey, you were totally right, I´ve added the comments and didn´t test afterward.