Skip to content

refactor(php): centralize the default PHP version into a single constant#238

Open
mrrobot47 wants to merge 2 commits into
EasyEngine:developfrom
mrrobot47:refactor/php-version-constant
Open

refactor(php): centralize the default PHP version into a single constant#238
mrrobot47 wants to merge 2 commits into
EasyEngine:developfrom
mrrobot47:refactor/php-version-constant

Conversation

@mrrobot47

Copy link
Copy Markdown
Member

Summary

Centralizes the PHP version into a single source of truth so a default bump is a one-line edit. Introduces two class constants on WordPress:

const DEFAULT_PHP_VERSION    = '8.4';
const SUPPORTED_PHP_VERSIONS = [ '5.6', …, '8.5', 'latest' ];

The default 8.4 was previously hardcoded independently in three spots; all now read DEFAULT_PHP_VERSION:

Consumer Before After
Default when --php omitted docblock default: 8.4 get_flag_value( …, 'php', self::DEFAULT_PHP_VERSION )
8.x unsupported-version fallback = 8.4; = self::DEFAULT_PHP_VERSION;
latest → image tag (Site_WP_Docker) 'easyengine/php8.4' 'easyengine/php' . WordPress::DEFAULT_PHP_VERSION

The inline $supported_php_versions array is replaced by SUPPORTED_PHP_VERSIONS.

Why removing the docblock default: works

EE injects a docblock default: into $assoc_args before create() runs, so a docblock default would always win over the constant. Removing it lets the get_flag_value() fallback supply DEFAULT_PHP_VERSION when --php is omitted — making the constant authoritative. Effective default is unchanged (8.4).

Also included

  • Indentation fix for the 8 === $floor branch (was mis-indented vs the 5.x/7.x branches).
  • String version literals instead of floats — fixes a latent bug where an x.0 version stringifies to "8" in the image name ('easyengine/php' . 8.0easyengine/php8).

Notes / tradeoffs for reviewers

  • --help no longer prints default: 8.4 for --php (the options list still shows). This is the inherent cost of single-sourcing the default in code. If preferred, the alternative is to keep the docblock default: and add a test asserting it equals the constant (drift-proof, but a bump becomes two edits).
  • README regeneration: because default: is gone from the docblock, ee scaffold package-readme will no longer emit the default: 8.4 line that docs: regenerate README for current PHP version support #237 added. Coordinate a README regen after merge (or keep the docblock default per the point above).
  • Irreducible duplication: the docblock options: list and prose still enumerate versions (PHPDoc can't reference a constant), so adding a new version still touches the docblock + SUPPORTED_PHP_VERSIONS. A synopsis↔constant guard test would make that drift-proof — happy to add it.

Introduce WordPress::DEFAULT_PHP_VERSION and WordPress::SUPPORTED_PHP_VERSIONS
as the single source of truth. The default value was previously hardcoded
independently in three places:

- the `--php` get_flag_value() default
- the 8.x unsupported-version fallback
- the `latest` -> image tag map in Site_WP_Docker

All three now read DEFAULT_PHP_VERSION, so moving the default is a one-line edit.

The `default:` line is removed from the `--php` docblock so the constant is the
authoritative default: EE injects a docblock default into the args only when one
is declared, so with it gone the get_flag_value() fallback supplies the constant.

Also fix the indentation of the 8.x fallback branch and use string literals for
the versions so an x.0 version no longer stringifies to "8" in the image name.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes the default/supported PHP versions for the ee site create --type=wp flow into WordPress class constants, so future default bumps are a single edit and consumers share the same source of truth.

Changes:

  • Introduces WordPress::DEFAULT_PHP_VERSION and WordPress::SUPPORTED_PHP_VERSIONS.
  • Uses DEFAULT_PHP_VERSION as the runtime default when --php is omitted and as the 8.x unsupported-version fallback.
  • Updates Site_WP_Docker so latest resolves to the Docker image tag matching DEFAULT_PHP_VERSION.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/WordPress.php Adds centralized version constants and wires them into --php defaulting/validation/fallback logic.
src/Site_WP_Docker.php Makes latest map to the Docker image tag derived from WordPress::DEFAULT_PHP_VERSION.
Comments suppressed due to low confidence (1)

src/WordPress.php:391

  • in_array() is currently non-strict for validating php_version. Because the supported list contains numeric-looking strings (e.g. '8.0'), a user input like '8' will be treated as supported via loose comparison and will skip the fallback, producing image keys like easyengine/php8 that likely don't exist. Using strict comparison and normalizing to string avoids this class of mismatch.
		$supported_php_versions = self::SUPPORTED_PHP_VERSIONS;
		if ( ! in_array( $this->site_data['php_version'], $supported_php_versions ) ) {
			$old_version = $this->site_data['php_version'];
			$floor       = (int) floor( $this->site_data['php_version'] );
			if ( 5 === $floor ) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/WordPress.php
…ation

Address review feedback on the version-constant refactor:

- Use a strict in_array() check for the supported-version test. Now that
  SUPPORTED_PHP_VERSIONS holds strings and php_version is always a string at
  that point, strict comparison avoids loose numeric matches (e.g. '8' loosely
  matching '8.0') that would otherwise skip the fallback and yield a
  non-existent image tag; such input now routes to the unsupported-version
  fallback instead.
- Normalize the 8.5 entry in the --php options list to a tab to match the
  other entries (it used spaces), keeping the docblock consistent.
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.

2 participants