007revad / Synology_M2_volume

Easily create an M.2 volume on Synology NAS
MIT License
810 stars 56 forks source link

Kudos! #61

Closed mceachen closed 1 year ago

mceachen commented 1 year ago

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 1 year ago

(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.

mceachen commented 1 year ago

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.