Skip to content

fix(php-update): remove underscore prefixes, fix multi-version summary messages - #227

Merged
PDowney merged 3 commits into
masterfrom
copilot/rename-seen-old-php-vers
May 2, 2026
Merged

fix(php-update): remove underscore prefixes, fix multi-version summary messages#227
PDowney merged 3 commits into
masterfrom
copilot/rename-seen-old-php-vers

Conversation

Copilot AI commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Software Version Updates

Variable naming inconsistencies and inaccurate summary messages in scripts/update/php-update.sh when multiple old PHP versions are detected.

Changed Versions

scripts/update/php-update.sh

  • _SEEN_OLD_PHP_VERS → SEEN_OLD_PHP_VERS — leading underscore was inconsistent with OLD_PHP_VERS / NEW_PHP_VER convention
  • _OLD_VER → OLD_VER in all four config-update loops (nginx php-fpm.conf, per-site configs, phpSysInfo, admin control panel API) — aligns with the removal loop that already used OLD_VER
  • Final summary header now uses ${MIGRATION_SOURCE_PHP_VERS[*]} instead of ${OLD_PHP_VER} — single-version reference was misleading when multiple old versions were migrated
  • "Removed PHP" bullet conditionally lists all removed versions via ${MIGRATION_SOURCE_PHP_VERS[*]}; falls back to single-version text only when the array is empty

Version Diff

-declare -A _SEEN_OLD_PHP_VERS=()
+declare -A SEEN_OLD_PHP_VERS=()

-    for _OLD_VER in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do
+    for OLD_VER in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do

-echo "PHP upgrade from ${OLD_PHP_VER} to ${NEW_PHP_VER} completed successfully."
+echo "PHP upgrade from ${MIGRATION_SOURCE_PHP_VERS[*]} to ${NEW_PHP_VER} completed successfully."

-echo "  - Removed PHP ${OLD_PHP_VER} installation"
+if [[ ${#MIGRATION_SOURCE_PHP_VERS[@]} -gt 0 ]]; then
+    echo "  - Removed PHP version(s): ${MIGRATION_SOURCE_PHP_VERS[*]}"
+else
+    echo "  - Removed PHP ${OLD_PHP_VER} installation"
+fi

Verification Checklist

  • Version numbers are correctly formatted
  • Version numbers are stable releases (not beta/RC)
  • No syntax errors in variables file

Notes

This is an automated pull request created by the software version checker workflow.
Please verify these versions are stable releases before merging.

Original prompt 
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The variable `_SEEN_OLD_PHP_VERS` uses a leading underscore which is typically reserved for private/internal variables, but this naming convention is inconsistent with other variables in the script (e.g., `OLD_PHP_VERS`, `NEW_PHP_VER`). Consider renaming to `SEEN_OLD_PHP_VERS` for consistency, or document why the underscore prefix is used here.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -36,12 +36,12 @@\n \n # Auto-detect currently installed PHP-FPM version\n OLD_PHP_VERS=()\n-declare -A _SEEN_OLD_PHP_VERS=()\n+declare -A SEEN_OLD_PHP_VERS=()\n for ver in \"${SUPPORTED_PHP_VERSIONS[@]}\"; do\n     if [[ \"${ver}\" != \"${NEW_PHP_VER}\" ]] && dpkg -l | grep -q \"php${ver}-fpm\"; then\n-        if [[ -z \"${_SEEN_OLD_PHP_VERS[\"${ver}\"]+x}\" ]]; then\n+        if [[ -z \"${SEEN_OLD_PHP_VERS[\"${ver}\"]+x}\" ]]; then\n             OLD_PHP_VERS+=(\"${ver}\")\n-            _SEEN_OLD_PHP_VERS[\"${ver}\"]=1\n+            SEEN_OLD_PHP_VERS[\"${ver}\"]=1\n         fi\n     fi\n done\n"}]},{"message":"The loop variable `_OLD_VER` uses a leading underscore, which is inconsistent with the naming convention used for `OLD_VER` in the removal loop (line 194). Consider using `OLD_VER` consistently throughout the script for clarity.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -133,16 +133,16 @@\n \n # Update php-fpm.conf\n if [[ -f \"/etc/nginx/globals/php-fpm.conf\" ]]; then\n-    for _OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-        sed -E -i \"s|php${_OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"/etc/nginx/globals/php-fpm.conf\"\n+    for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n+        sed -E -i \"s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"/etc/nginx/globals/php-fpm.conf\"\n     done\n fi\n \n # Update all nginx site configurations\n for config_file in /etc/nginx/sites-available/*; do\n     if [[ -f \"$config_file\" ]]; then\n-        for _OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-            sed -E -i \"s|php${_OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"$config_file\"\n+        for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n+            sed -E -i \"s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"$config_file\"\n         done\n     fi\n done\n@@ -150,8 +144,8 @@\n # Update phpSysInfo configuration\n if [[ -f \"/var/www/admin/tools/phpsysinfo/phpsysinfo.ini\" ]]; then\n     echo \"Updating phpSysInfo configuration...\"\n-    for _OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-        sed -E -i \"s|php${_OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"/var/www/admin/tools/phpsysinfo/phpsysinfo.ini\"\n+    for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n+        sed -E -i \"s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"/var/www/admin/tools/phpsysinfo/phpsysinfo.ini\"\n     done\n fi\n \n"}]},{"message":"The final message references only `${OLD_PHP_VER}` (the first detected version) instead of all migrated versions in `${MIGRATION_SOURCE_PHP_VERS[@]}`. When multiple old PHP versions are detected and removed, this message will be misleading. Use `${MIGRATION_SOURCE_PHP_VERS[*]}` to accurately reflect all versions that were migrated from.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -232,7 +232,7 @@\n echo \"\"\n echo \"=============================================================\"\n echo \"\"\n-echo \"PHP upgrade from ${OLD_PHP_VER} to ${NEW_PHP_VER} completed successfully.\"\n+echo \"PHP upgrade from ${MIGRATION_SOURCE_PHP_VERS[*]} to ${NEW_PHP_VER} completed successfully.\"\n echo \"\"\n echo \"Changes made:\"\n echo \"  - Installed PHP ${NEW_PHP_VER} and extensions\"\n"}]},{"message":"Similar to the previous issue, this message only mentions `${OLD_PHP_VER}` but the script may have removed multiple PHP versions. The message should reflect all versions removed by referencing `${MIGRATION_SOURCE_PHP_VERS[*]}` or indicate that multiple versions were removed.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -238,7 +238,11 @@\n echo \"  - Installed PHP ${NEW_PHP_VER} and extensions\"\n echo \"  - Applied EngineScript configuration for PHP ${NEW_PHP_VER}\"\n echo \"  - Updated Nginx configuration\"\n-echo \"  - Removed PHP ${OLD_P...

@PDowney
fix(php-update): remove inconsistent underscore prefixes and fix mult…
3af1488 
…i-version summary messages

Agent-Logs-Url: https://github.com/EngineScript/EngineScript/sessions/8362710e-d6fe-4a08-b69d-90fb9d3b4168

Co-authored-by: PDowney <11467177+PDowney@users.noreply.github.com>
Copilot AI changed the title [WIP] Rename variable for consistency in php-update.sh fix(php-update): remove underscore prefixes, fix multi-version summary messages May 2, 2026
Copilot AI requested a review from PDowney May 2, 2026 03:26
Copilot finished work on behalf of PDowney May 2, 2026 03:26
@github-actions

github-actions Bot commented May 2, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@sonarqubecloud

sonarqubecloud Bot commented May 2, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@PDowney
PDowney marked this pull request as ready for review May 2, 2026 04:09
Copilot AI review requested due to automatic review settings May 2, 2026 04:09
@PDowney
PDowney merged commit 779f0a8 into master May 2, 2026
11 checks passed
@github-actions
github-actions Bot deleted the copilot/rename-seen-old-php-vers branch May 2, 2026 04:09
Copilot AI reviewed May 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates scripts/update/php-update.sh, the PHP migration script used by EngineScript’s server-maintenance workflow, to make multi-version upgrade reporting more accurate and to normalize some variable names.

Changes:

  • Renames the associative-array tracking variable used during old PHP version detection for consistency.
  • Renames loop variables in the config migration sections to match the rest of the script.
  • Updates the final upgrade summary so it reports all migrated/removed source PHP versions instead of only a single legacy variable.

Comment thread scripts/update/php-update.sh
Comment on lines +241 to +245
if [[ ${#MIGRATION_SOURCE_PHP_VERS[@]} -gt 0 ]]; then
echo " - Removed PHP version(s): ${MIGRATION_SOURCE_PHP_VERS[*]}"
else
echo " - Removed PHP ${OLD_PHP_VER} installation"
fi
Comment thread scripts/update/php-update.sh
echo "============================================================="
echo ""
echo "PHP upgrade from ${OLD_PHP_VER} to ${NEW_PHP_VER} completed successfully."
echo "PHP upgrade from ${MIGRATION_SOURCE_PHP_VERS[*]} to ${NEW_PHP_VER} completed successfully."
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@PDowney PDowney PDowney approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@PDowney PDowney

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@PDowney