Dotenv #21
Dotenv #21
Conversation
| * from it | ||
| */ | ||
|
|
||
| // 'dotenv_path' => __DIR__, |
There was a problem hiding this comment.
I prefer the __DIR__ default. Can you restore this? I think it'd be better to have the default, and then do a file_exists before loading Dotenv.
| @@ -292,6 +293,11 @@ protected function loadConfig() | |||
| define('CRAFT_TRANSLATIONS_PATH', rtrim($config['craft_translations_path'], '/').'/'); | |||
| } | |||
|
|
|||
| if (isset($config['dotenv_path'])) { | |||
There was a problem hiding this comment.
I'd like to see a default dotenv_path set here in case the user does not have a .craft-cli.php config file.
$dotenvPath = isset($config['dotenv_path']) ? $config['dotenv_path'] : getcwd();
This is also where you should add the file exists check so we can have a default dotenv_path:
if (file_exists($dotenvPath.'/.env')) {
$dotenv = new Dotenv($dotenvPath);
$dotenv->load();
}
| @@ -292,6 +293,11 @@ protected function loadConfig() | |||
| define('CRAFT_TRANSLATIONS_PATH', rtrim($config['craft_translations_path'], '/').'/'); | |||
| } | |||
|
|
|||
| if (isset($config['dotenv_path'])) { | |||
There was a problem hiding this comment.
Also, can you move the Dotenv loading above this line, before the $environment variable is set: https://github.com/tomdavies/craft-cli/blob/4ad9653e4b0b8b6deb5c1ebdd2cad2fa01c7720e/src/Application.php#L270
|
I left a few comments regarding some changes I'd like to see before merging. I'd be happy to make the changes myself if you give me write access to your Pull Request (see this guide: https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/). The purpose of these changes is so that someone can add a |
|
Hey, I'm happy to make all these tonight (overnight your TZ)
|
* master: add documentation for dotenv usage amends from @rsanchez's code review Add phpdotenv support
|
@rsanchez amends per your eminently sensible comments. You have write access to my PR branch if you want to do anything further - feel free. |
As previously discussed in #20