007revad/Synology_M2_volume

Kudos!

mceachen opened this issue · 2 comments

mceachen commented

Thanks for this work!

In case you've not seen it before, shellcheck is a great linter for shell, and integrates with a bunch of editors (I use the this extension with vscode, fwiw).

It didn't find anything horrible here--just a few nits:

 shellcheck syno_create_m2_volume.sh 

In syno_create_m2_volume.sh line 79:
[ "$BASH" ] && ! shopt -qo posix || {
            ^-- SC2015 (info): Note that A && B || C is not if-then-else. C may run when A is true.


In syno_create_m2_volume.sh line 127:
    minor=$(get_key_value /etc.defaults/VERSION minor)
    ^---^ SC2034 (warning): minor appears unused. Verify use (or export if used externally).


In syno_create_m2_volume.sh line 151:
args="$@"
     ^--^ SC2124 (warning): Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.


In syno_create_m2_volume.sh line 176:
                log=yes
                ^-^ SC2034 (warning): log appears unused. Verify use (or export if used externally).


In syno_create_m2_volume.sh line 179:
                debug=yes
                ^---^ SC2034 (warning): debug appears unused. Verify use (or export if used externally).


In syno_create_m2_volume.sh line 216:
    dsm72="yes"
    ^---^ SC2034 (warning): dsm72 appears unused. Verify use (or export if used externally).


In syno_create_m2_volume.sh line 348:
                                delerr=1
                                ^----^ SC2034 (warning): delerr appears unused. Verify use (or export if used externally).

For more information:
  https://www.shellcheck.net/wiki/SC2034 -- debug appears unused. Verify use ...
  https://www.shellcheck.net/wiki/SC2124 -- Assigning an array to a string! A...
  https://www.shellcheck.net/wiki/SC2015 -- Note that A && B || C is not if-t...

Feel free to close this issue--I just wanted to stop by and thank you for your work.

(I found this project via https://www.mklibrary.com/synology-nvme-volume/)

Cheers!

007revad commented

(I found this project via https://www.mklibrary.com/synology-nvme-volume/)

That's a well written guide. I might put a link to it on my readme page.

I do use shellcheck.

  • The args="$@" is just so I can echo the arguments used. But I could change it args=("$@")
  • The unused variables are for future use.
  • Line 79 is something that does work when the script is in a different shell 9to check it's running in bash) even though shellcheck warns that it's not an "if, then, else".
  • 👍1
mceachen commented

ICYMI, you can prepend the line # shellcheck disable=SC2015 (or whatever the warning is) right above the offending line of code if you've checked out the linting error and have determined it's a false positive or otherwise ignorable. Docs are here.

Thanks again.

  • 👍1