[WIP] Add support for generating env specific config caches#219
[WIP] Add support for generating env specific config caches#219
Conversation
Might it not rather be:
|
You are 100% correct, was in a rush to pick up my car from the garage, thank you |
There was a problem hiding this comment.
Pull Request Overview
This PR implements Laravel-style config:cache and config:clear commands for Winter CMS with environment-specific configuration caching. The implementation allows generating cached configs for specific environments (e.g., config:cache banana) while maintaining backward compatibility with default behavior.
Key changes:
- Environment-specific cached config file naming (e.g.,
production.config.php,banana.config.php) - Support for environment arguments in both cache and clear commands
- Modified configuration loading to check for cached configs before falling back to file-based loading
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/Foundation/Application.php |
Updates cached config path to include environment name |
src/Foundation/Console/ConfigCacheCommand.php |
Replaces warning with full implementation supporting environment-specific caching |
src/Foundation/Console/ConfigClearCommand.php |
Replaces warning with implementation to clear environment-specific cached configs |
src/Foundation/Bootstrap/LoadConfiguration.php |
Adds logic to load from cached config files when available |
src/Config/Repository.php |
Modifies constructor to accept pre-loaded config items |
tests/Foundation/ApplicationTest.php |
Updates tests to reflect new environment-specific config paths |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| $configPath = realpath( | ||
| dirname($this->laravel->getCachedConfigPath()) | ||
| . DIRECTORY_SEPARATOR | ||
| . $this->argument('env') | ||
| . '.config.php' | ||
| ); |
There was a problem hiding this comment.
The realpath() function will return false if the file doesn't exist, which could cause issues when trying to delete a non-existent config file. Consider using dirname($this->laravel->getCachedConfigPath()) . DIRECTORY_SEPARATOR . $this->argument('env') . '.config.php' directly without realpath().
| $configPath = realpath( | |
| dirname($this->laravel->getCachedConfigPath()) | |
| . DIRECTORY_SEPARATOR | |
| . $this->argument('env') | |
| . '.config.php' | |
| ); | |
| $configPath = dirname($this->laravel->getCachedConfigPath()) | |
| . DIRECTORY_SEPARATOR | |
| . $this->argument('env') | |
| . '.config.php'; |
| catch (Exception $ex) { | ||
| // | ||
| } |
There was a problem hiding this comment.
Empty catch blocks make debugging difficult. Consider logging the exception or adding a comment explaining why the exception is being silently ignored.
| catch (Exception $ex) { | ||
| // | ||
| } |
There was a problem hiding this comment.
Empty catch blocks make debugging difficult. Consider logging the exception or adding a comment explaining why the exception is being silently ignored.
|
This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. |
|
This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days. |
This PR implements support for Laravel's
config:cache&config:clearcommands.The configs generated are env specific, which means that if you have
config/banana/app.phpwithbananaspecific configs such as in a multi site setup you can useconfig:cache bananato specifically generate a cached config for that env.By default, the env used will be the one provided via
APP_ENV, or if not setenvironments.default, or if not set the valueproduction.Usage
php artisan config:cachephp artisan config:cache bananaphp artisan config:clearphp artisan config:clear bananaTODO
If anybody from wintercms/winter#1297 (comment) or wintercms/winter#885 (comment) would like to help test, that would be massively appreciated.