-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ADD: WP constants in .env & application.php #680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
There are some issues that would need to be updated before we'd be able to merge this.
I've added some comments. I would also encourage you to follow our code style, but that's something we can review later.
I'll defer to @swalkinshaw on whether this is the direction we want to take with Bedrock. My personal take is that this PR is making too many changes, and we should be having a discussion about each of these different settings instead of adding all of them in a single PR.
# Configuring development mode | ||
# Values: core, plugin, theme, all (default) | ||
# See: https://make.wordpress.org/core/2023/07/14/configuring-development-mode-in-6-3/ | ||
# WP_DEVELOPMENT_MODE=core | ||
|
||
## WP OPTIONS | ||
WP_CACHE=true | ||
WP_POST_REVISIONS=5 | ||
WP_CONCATENATE_SCRIPTS=false | ||
WP_COMPRESS_SCRIPTS=true | ||
WP_COMPRESS_CSS=true | ||
WP_ENFORCE_GZIP=true | ||
DISABLE_WP_CRON=false | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably don't need any of this here if we have sensible production-ready defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get rid of this file.
@@ -7,6 +7,7 @@ | |||
use function Env\env; | |||
|
|||
Config::define('SAVEQUERIES', true); | |||
Config::define('WP_DEVELOPMENT_MODE', env('WP_DEVELOPMENT_MODE') ?? 'all'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have my reservations about this.
@@ -17,6 +17,20 @@ WP_SITEURL="${WP_HOME}/wp" | |||
# Specify optional debug.log path | |||
# WP_DEBUG_LOG='/path/to/debug.log' | |||
|
|||
# Configuring development mode | |||
# Values: core, plugin, theme, all (default) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that default is an empty string, not all
An empty string indicates that no particular development mode is enabled for this site. This is the default value and should be used on any site that is not used for development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, my bad, I've coded it too fast! ^^'
Config::define( 'COMPRESS_CSS', env('WP_COMPRESS_CSS') ?? false ); | ||
|
||
// Force GZIP compression (default is deflate) | ||
Config::define( 'ENFORCE_GZIP', env('WP_ENFORCE_GZIP') ?? 'deflate' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is interesting, but I would rather this not be handled by the application. This is something that should be handled by the server.
Add some WP constants and default value if not defined in .env file.