Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DSGND
Copy link
Contributor

@DSGND DSGND commented Jun 12, 2023

Add some WP constants and default value if not defined in .env file.

Copy link
Sponsor Member

@QWp6t QWp6t left a 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.

Comment on lines +20 to +33
# 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

Copy link
Sponsor Member

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.

Copy link
Sponsor Member

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');
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -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)
Copy link
Sponsor Member

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.

Copy link
Contributor Author

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' );
Copy link
Sponsor Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants