refactor(php): centralize the default PHP version into a single constant#113
Open
mrrobot47 wants to merge 2 commits into
Open
refactor(php): centralize the default PHP version into a single constant#113mrrobot47 wants to merge 2 commits into
mrrobot47 wants to merge 2 commits into
Conversation
Introduce PHP::DEFAULT_PHP_VERSION and PHP::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_PHP_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 use string literals for the versions so an x.0 version no longer stringifies to "8" in the image name.
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Centralizes the PHP version into a single source of truth so a default bump is a one-line edit. Mirrors the same change in site-type-wp (EasyEngine/site-type-wp#238).
Introduces two class constants on
PHP:The default
8.4was previously hardcoded independently in three spots; all now readDEFAULT_PHP_VERSION:--phpomitteddefault: 8.4get_flag_value( …, 'php', self::DEFAULT_PHP_VERSION )= 8.4;= self::DEFAULT_PHP_VERSION;latest→ image tag (Site_PHP_Docker)'easyengine/php8.4''easyengine/php' . PHP::DEFAULT_PHP_VERSIONThe inline
$supported_php_versionsarray is replaced bySUPPORTED_PHP_VERSIONS.Why removing the docblock
default:worksEE injects a docblock
default:into$assoc_argsbeforecreate()runs, so a docblock default would always win over the constant. Removing it lets theget_flag_value()fallback supplyDEFAULT_PHP_VERSIONwhen--phpis omitted — making the constant authoritative. Effective default is unchanged (8.4).Also included
x.0version stringifies to"8"in the image name ('easyengine/php' . 8.0→easyengine/php8).Notes / tradeoffs for reviewers
--helpno longer printsdefault: 8.4for--php(the options list still shows). Inherent cost of single-sourcing the default in code. Alternative: keep the docblockdefault:and add a test asserting it equals the constant (drift-proof, but a bump becomes two edits).default:is gone from the docblock, soee scaffold package-readmewill no longer emit thedefault: 8.4line that docs: regenerate README for current PHP version support #112 added. Coordinate a README regen after merge (or keep the docblock default per the point above).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.