Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 consideredobsolete 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.