babelfish-for-postgresql / babelfish_compass

Babelfish Compass: compatibility assessment tool for Babelfish for PostgreSQL
Apache License 2.0
108 stars 21 forks source link

bash script style suggestions #49

Closed max-webster closed 1 year ago

max-webster commented 2 years ago

A few suggested changes for portability / modernization / robustness of the BabelfishCompass.sh script:

I changed the shebang argument to '/usr/bin/env bash' for portability. On some Mac and Linux systems, I've observed the preferred bash being in /usr/bin, /usr/local/bin, or other locations other than /bin. The /usr/bin/env idiom uses the first bash that is found in $PATH. Since the script relies on $PATH to locate the 'java' executable, I presume that type of flexibility is OK for this script.

Along the same lines, I used 'which' to determine the full path to the java executable and assign that to ${JAVA}. To make debugging simpler if ${JAVA} needs to be echoed later, e.g. to figure out if the right JDK/JRE is being used.

I used $(...) notation instead of ... throughout. Backticks are considered obsolete from a POSIX perspective. See http://mywiki.wooledge.org/BashFAQ/082 or https://unix.stackexchange.com/questions/126927/have-backticks-i-e-cmd-in-sh-shells-been-deprecated for why $() is preferable.

I added quotes around "$?", for consistency with other 'if [[ ]]' arguments in the script.

I changed sed to tr in the pipeline stage that deletes double quotes. Just to simplify the action of getting rid of such-and-such characters, without introducing the overhead of a regexp search and replace.

I changed $* to "$@" where shellscript arguments are passed through to 'java'. That's a change recommended by the 'shellcheck' linter utility, to avoid problems if some of the arguments contain spaces.

Signed-off-by: John Russell johrss@amazon.com

Description

Makes the script a little more portable (especially for Mac), maintainable, robust. Reduces warnings from 'shellcheck' utility.

Issues Resolved

[List any issues this PR will resolve]

Check List

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.