Fix quoting and escaping shellcheck errors #38

Manually merged
Ghost merged 2 commits from shellcheck-escapes into master 2020-04-21 15:05:13 +02:00
First-time contributor

shellcheck was still complaining about a few SC1117 and SC2086
warnings. I ignored those that did not seem necessary and fixed the
rest. The less linter noise the better.

I need validation that I understood these checks correctly, I'll be testing them in a few minutes.

shellcheck was still complaining about a few SC1117 and SC2086 warnings. I ignored those that did not seem necessary and fixed the rest. The less linter noise the better. I need validation that I understood these checks correctly, I'll be testing them in a few minutes.
Ghost added the
zzz_evobackup
label 2020-04-15 20:54:17 +02:00
jlecour was assigned by Ghost 2020-04-15 20:54:17 +02:00
jlecour reviewed 2020-04-16 08:14:57 +02:00
jlecour left a comment
Owner

I'm not sure printf likes to see escapes line breaks. Did you test this? Did it work?

I'm not sure printf likes to see escapes line breaks. Did you test this? Did it work?
jlecour reviewed 2020-04-16 08:22:20 +02:00
jlecour left a comment
Owner

I've added some inline comments

I've added some inline comments
@ -84,3 +84,3 @@
# SSH connection failed
new_error=$(printf "Failed to connect to \`%s' within %s seconds" "${item}" "${SSH_CONNECT_TIMEOUT}")
SERVERS_SSH_ERRORS=$(printf "%s\n%s" "${SERVERS_SSH_ERRORS}" "${new_error}" | sed -e '/^$/d')
SERVERS_SSH_ERRORS=$(printf "%s\\n%s" "${SERVERS_SSH_ERRORS}" "${new_error}" | sed -e '/^$/d')
Owner

I'm not sure printf likes to see escaped line breaks. Did you test this? Did it work?

I'm not sure printf likes to see escaped line breaks. Did you test this? Did it work?
Author
First-time contributor

I've just tested manually and it worked, we will see tomorrow if it worked when run from cron.

I've just tested manually and it worked, we will see tomorrow if it worked when run from cron.
zzz_evobackup Outdated
@ -123,6 +123,8 @@ pick_server() {
if [ -e "${PIDFILE}" ]; then
pid=$(cat "${PIDFILE}")
# Does process still exist ?
# ignore check because multiple processes might exist
Owner

AFAIK only a single PID can be in this pidfile, so we can surrounf the variable with double-quotes and not ignore the warning

AFAIK only a single PID can be in this pidfile, so we can surrounf the variable with double-quotes and not ignore the warning
Author
First-time contributor

Fixed

Fixed
zzz_evobackup Outdated
@ -367,2 +369,4 @@
RSH_COMMAND="ssh -p ${SSH_PORT} -o 'ConnectTimeout ${SSH_CONNECT_TIMEOUT}'"
# ignore check because we want it to split the different arguments to $rep
# shellcheck disable=SC2086
Owner

This one is OK

This one is OK
Author
First-time contributor

If everything is okay tomorrow, I will merge.

If everything is okay tomorrow, I will merge.
Author
First-time contributor

Everything works just fine with the escaped characters and the quoted variable.

Everything works just fine with the escaped characters and the quoted variable.
Ghost closed this pull request 2020-04-21 15:05:13 +02:00
Ghost deleted branch shellcheck-escapes 2020-04-21 15:05:30 +02:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: evolix/evobackup#38
No description provided.