Skip to content

Conversation

kojiromike
Copy link
Contributor

@kojiromike kojiromike commented Jul 1, 2025

Short description of what this resolves:

Fixes all remaining shellcheck findings

Changes proposed in this pull request:

Mostly quotes, but also a few default or required variable values, and where it's easy, make scripts consistent across versions.

@jesdynf
Copy link
Collaborator

jesdynf commented Jul 2, 2025

@bradymiller I don't think this is exceptional and I've spot-checked it (and tested things on the commandline until I got what it was trying to get across) but it touches every script we own. Is there anything you want to peek at it before we let it through?

@@ -42,7 +42,7 @@ auto_setup() {
echo "opcache.max_accelerated_files=1000000" >> auto_configure.ini

#run auto_configure
php auto_configure.php -c auto_configure.ini -f ${CONFIGURATION} || return 1
php auto_configure.php -c auto_configure.ini -f "${CONFIGURATION}" || return 1
Copy link
Member

Choose a reason for hiding this comment

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

Will this require this change (both for 7.0.3 and 7.0.4) if not already there:

// Collect parameters(if exist) for installation configuration settings
for ($i=1; $i < count($argv); $i++) {
if ($argv[$i] == '-f' && isset($argv[$i+1])) {
// Handle case where a single string contains all parameters
$configString = $argv[$i+1];
$configPairs = preg_split('/\s+/', trim($configString));
foreach ($configPairs as $pair) {
if (strpos($pair, '=') !== false) {
list($index, $value) = explode('=', $pair, 2);
$installSettings[$index] = $value;
}
}
// Skip the next argument since we already processed it
$i++;
} else {
// Handle standard key=value parameters
$indexandvalue = explode("=", $argv[$i]);
$index = $indexandvalue[0];
$value = $indexandvalue[1] ?? '';
$installSettings[$index] = $value;
}
}

@kojiromike kojiromike changed the title fix(shell): SC2086 auto apply fix(shell): all remaining shellcheck issues Jul 18, 2025
@kojiromike
Copy link
Contributor Author

bah, there was a bug in my git ls-files and this didn't actually catch all the shellcheck issues. There are still a ton in the xbackup scripts.

@jesdynf
Copy link
Collaborator

jesdynf commented Jul 18, 2025

Those might not be worth fighting. I need to migrate away from the current MySQL image with Percona's XtraBackup stuff to something with MariaDB.

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.

3 participants