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

[BUGS-7620] define WP_HOME #115

Merged
merged 13 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ jobs:

- name: Cache Composer dependencies
id: composer-cache
run: echo "::set-output name=dir::$(composer config cache-files-dir)"
run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT

- uses: actions/cache@v2
- uses: actions/cache@v4
with:
path: ${{ steps.composer-cache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
Expand Down
42 changes: 42 additions & 0 deletions config/application.pantheon.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php
/**
* Pantheon platform application settings.
*
* IMPORTANT NOTE:
* Do not modify this file. This file is maintained by Pantheon.
Copy link
Member

Choose a reason for hiding this comment

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

Usually, Pantheon-managed files in a composer-managed site are brought in via a dependency, e.g. pantheon-systems/drupal-integrations for Drupal sites. For WordPress composer-managed sites, this pattern was not followed because there were not many files to manage this way. Is there any appetite to revisit this decision?

Copy link
Contributor Author

@jazzsequence jazzsequence Apr 9, 2024

Choose a reason for hiding this comment

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

Ultimately, I don't know that that would leave us in a better place than adding a file for Pantheon-maintained modifications as far as merge conflicts. Currently, we're making a one-line change in a part of application.php that is unlikely to change much, if they've changed the file at all. If we added a dependency and moved our modifications there, we'd make changes to the composer.json, which users almost certainly have made modifications to.

Also, environment-specific application.php files is a recommended practice for Bedrock. We're going against the grain a little bit by not using the /environment/ directory, but since our changes apply to the platform and not a specific environment, this seemed to make the most sense. If people are familiar with Bedrock already, having a Pantheon-specific config file won't seem out of place.

I'm not broadly opposed to the idea, but this is also code that would otherwise have already existed in wp-config.php (or wp-config-pantheon.php) and I think it would be weirder to not make it part of the core project.

*
* Site-specific modifications belong in config/application.php, not this file.
* This file may change in future releases and modifications would cause
* conflicts when attempting to apply upstream updates.
*/

use Roots\WPConfig\Config;
use function Env\env;

if ( isset( $_ENV['PANTHEON_ENVIRONMENT'] ) && 'lando' !== $_ENV['PANTHEON_ENVIRONMENT'] ) {
jazzsequence marked this conversation as resolved.
Show resolved Hide resolved
// We can use PANTHEON_SITE_NAME here because it's safe to assume we're on a Pantheon environment if PANTHEON_ENVIRONMENT is set.
$sitename = $_ENV['PANTHEON_SITE_NAME'];
$baseurl = $_ENV['PANTHEON_ENVIRONMENT'] . '-' . $sitename . '.pantheonsite.io';

$scheme = 'http';
if ( isset( $_SERVER['HTTPS'] ) && 'on' === $_SERVER['HTTPS'] ) {
$scheme = 'https';
}

// Define the WP_HOME and WP_SITEURL constants if they aren't already defined.
if ( ! env( 'WP_HOME' ) ) {
// If HTTP_HOST is set, use that as the base URL. It's probably more accurate.
if ( isset( $_SERVER['HTTP_HOST'] ) ) {
$baseurl = $_SERVER['HTTP_HOST'];
}

$homeurl = $scheme . '://' . $baseurl;
Config::define( 'WP_HOME', $homeurl );
putenv( 'WP_HOME=' . $homeurl );

if ( ! env( 'WP_SITEURL' ) ) {
Config::define( 'WP_SITEURL', $homeurl . '/wp' );
putenv( 'WP_SITEURL=' . $homeurl . '/wp' );
}
}
}
5 changes: 5 additions & 0 deletions config/application.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
}
}

/**
* Include Pantheon application settings.
*/
require_once __DIR__ . '/application.pantheon.php';

/**
* Set up our global environment constant and load its config first
* Default: production
Expand Down
9 changes: 9 additions & 0 deletions phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,18 @@
<exclude name="WordPress.WP.GlobalVariablesOverride.Prohibited">
<exclude-pattern>config/application.php</exclude-pattern>
</exclude>
<exclude name="WordPress.Files.FileName.NotHyphenatedLowercase">
<exclude-pattern>config/application.pantheon.php</exclude-pattern>
</exclude>
</rule>
<!-- End Pantheon addition -->

<!--
Prevent errors caused by WordPress Coding Standards not supporting PHP 8.0+.
See https://github.com/WordPress/WordPress-Coding-Standards/issues/2035
-->
<ini name="error_reporting" value="E_ALL &#38; ~E_DEPRECATED" />

<!-- Show colors in console -->
<arg value="-colors"/>

Expand Down
Loading