refactor: combine duplicate loops in admin-control-panel-install.sh - #224
Conversation
Agent-Logs-Url: https://github.com/EngineScript/EngineScript/sessions/a29122e3-0285-4abb-82af-093779d5b6fa Co-authored-by: PDowney <11467177+PDowney@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
There was a problem hiding this comment.
Pull request overview
Refactors the admin control panel install script to reduce duplication by performing dashboard version placeholder substitution and verification in a single loop.
Changes:
- Combine the
sedsubstitution and{ES_DASHBOARD_VER}placeholder verification into one per-file loop. - Add a clarifying comment describing the combined substitution + verification pass.
| for file in index.html dashboard.js; do | ||
| sed -i "s|{ES_DASHBOARD_VER}|${ES_DASHBOARD_VER}|g" "/var/www/admin/control-panel/${file}" | ||
| done | ||
|
|
||
| # Verify that the dashboard placeholder was successfully replaced to avoid silent failures | ||
| for file in index.html dashboard.js; do | ||
| if grep -q '{ES_DASHBOARD_VER}' "/var/www/admin/control-panel/${file}"; then | ||
| echo "Error: Failed to substitute dashboard version in /var/www/admin/control-panel/${file}; placeholder {ES_DASHBOARD_VER} still present." >&2 |
There was a problem hiding this comment.
grep -q inside the if will silently treat missing/unreadable files as “no match” (grep exits 2), so placeholder verification can be skipped without failing the install. Consider explicitly checking that each target file exists/readable before running sed/grep, and/or handling grep’s exit status to error on I/O failures.
| # Substitute and verify dashboard version placeholders in one pass | ||
| for file in index.html dashboard.js; do |
There was a problem hiding this comment.
Repo standard is to record changes in the root CHANGELOG.md when modifying the codebase (see .github/copilot-instructions.md “Documentation & Change Tracking”). This PR changes an install script but doesn’t include a changelog entry.
Software Version Updates
The
for file in index.html dashboard.jsloop was duplicated — once forsedsubstitution, once for placeholder verification. Merged into a single pass.Changed Versions
scripts/install/tools/frontend/admin-control-panel-install.sh: combined substitution + verification into one loopVersion Diff
Verification Checklist
Notes
No behavioural change. Pure deduplication — substitution and verification now happen per-file in the same iteration rather than two separate full passes.
Original prompt