From 51eab627bf72a3dfa359ea6110ea4aabd2be3d13 Mon Sep 17 00:00:00 2001 From: Lucas Kolstad Date: Mon, 31 Jul 2017 13:49:55 -0700 Subject: [PATCH] Address code review on commit e7df3211f597b. Add braces around all quoted Bash variables. Uncomment the lines that were erroneously commented out in publish.sh that check for uncommitted changes. Cleanup loop style and remove an unneeded option in a call to the find command. --- examples/todo/bootstrap.sh | 6 +++--- scripts/mk-docs.sh | 10 +++++----- scripts/publish.sh | 10 +++++----- scripts/test.sh | 13 ++++++------- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/examples/todo/bootstrap.sh b/examples/todo/bootstrap.sh index 5a58ec2c..f2307073 100755 --- a/examples/todo/bootstrap.sh +++ b/examples/todo/bootstrap.sh @@ -3,9 +3,9 @@ SCRIPT_PATH=$(cd "$(dirname "$0")" ; pwd -P) DATABASE_URL="${SCRIPT_PATH}/db/db.sql" -pushd "$SCRIPT_PATH" > /dev/null +pushd "${SCRIPT_PATH}" > /dev/null # clear an existing database - rm -f "$DATABASE_URL" + rm -f "${DATABASE_URL}" # install the diesel CLI tools if they're not installed if ! command -v diesel >/dev/null 2>&1; then @@ -13,7 +13,7 @@ pushd "$SCRIPT_PATH" > /dev/null fi # create db/db.sql - diesel migration --database-url="$DATABASE_URL" run > /dev/null + diesel migration --database-url="${DATABASE_URL}" run > /dev/null popd > /dev/null echo "export DATABASE_URL=\"${DATABASE_URL}\"" diff --git a/scripts/mk-docs.sh b/scripts/mk-docs.sh index 42c72ead..4372a182 100755 --- a/scripts/mk-docs.sh +++ b/scripts/mk-docs.sh @@ -7,12 +7,12 @@ set -e # Brings in: ROOT_DIR, EXAMPLES_DIR, LIB_DIR, CODEGEN_DIR, CONTRIB_DIR, DOC_DIR SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -source "$SCRIPT_DIR/config.sh" +source "${SCRIPT_DIR}/config.sh" function mk_doc() { local dir=$1 local flag=$2 - pushd "$dir" > /dev/null 2>&1 + pushd "${dir}" > /dev/null 2>&1 echo ":: Documenting '${dir}'..." cargo doc --no-deps --all-features popd > /dev/null 2>&1 @@ -21,9 +21,9 @@ function mk_doc() { # We need to clean-up beforehand so we don't get all of the dependencies. cargo clean -mk_doc "$LIB_DIR" -mk_doc "$CODEGEN_DIR" -mk_doc "$CONTRIB_DIR" +mk_doc "${LIB_DIR}" +mk_doc "${CODEGEN_DIR}" +mk_doc "${CONTRIB_DIR}" # Blank index, for redirection. touch "${DOC_DIR}/index.html" diff --git a/scripts/publish.sh b/scripts/publish.sh index 67f7db4c..87bff375 100755 --- a/scripts/publish.sh +++ b/scripts/publish.sh @@ -7,12 +7,12 @@ set -e # Brings in: ROOT_DIR, EXAMPLES_DIR, LIB_DIR, CODEGEN_DIR, CONTRIB_DIR, DOC_DIR SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -source "$SCRIPT_DIR/config.sh" +source "${SCRIPT_DIR}/config.sh" -#if ! [ -z "$(git status --porcelain)" ]; then -# echo "There are uncommited changes! Aborting." -# exit 1 -#fi +if ! [ -z "$(git status --porcelain)" ]; then + echo "There are uncommited changes! Aborting." + exit 1 +fi # Ensure everything passes before trying to publish. echo ":::: Running test suite..." diff --git a/scripts/test.sh b/scripts/test.sh index 0215a45b..e353d943 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -3,7 +3,7 @@ set -e # Brings in: ROOT_DIR, EXAMPLES_DIR, LIB_DIR, CODEGEN_DIR, CONTRIB_DIR, DOC_DIR SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -source "$SCRIPT_DIR/config.sh" +source "${SCRIPT_DIR}/config.sh" # Add Cargo to PATH. export PATH=${HOME}/.cargo/bin:${PATH} @@ -11,7 +11,7 @@ export PATH=${HOME}/.cargo/bin:${PATH} # Checks that the versions for Cargo projects $@ all match function check_versions_match() { local last_version="" - for dir in "$@"; do + for dir in "${@}"; do local cargo_toml="${dir}/Cargo.toml" if ! [ -f "${cargo_toml}" ]; then echo "Cargo configuration file '${cargo_toml}' does not exist." @@ -31,7 +31,7 @@ function check_versions_match() { # Ensures there are no tabs in any file. function ensure_tab_free() { local tab=$(printf '\t') - local matches=$(grep -I -R "${tab}" "$ROOT_DIR" | egrep -v '/target|/.git|LICENSE') + local matches=$(grep -I -R "${tab}" "${ROOT_DIR}" | egrep -v '/target|/.git|LICENSE') if ! [ -z "${matches}" ]; then echo "Tab characters were found in the following:" echo "${matches}" @@ -41,7 +41,7 @@ function ensure_tab_free() { # Ensures there are no files with trailing whitespace. function ensure_trailing_whitespace_free() { - local matches=$(egrep -I -R " +$" "$ROOT_DIR" | egrep -v "/target|/.git") + local matches=$(egrep -I -R " +$" "${ROOT_DIR}" | egrep -v "/target|/.git") if ! [ -z "${matches}" ]; then echo "Trailing whitespace was found in the following:" echo "${matches}" @@ -50,8 +50,7 @@ function ensure_trailing_whitespace_free() { } function bootstrap_examples() { - while read -r file; - do + while read -r file; do bootstrap_script="${file}/bootstrap.sh" if [ -x "${bootstrap_script}" ]; then echo " Bootstrapping ${file}..." @@ -65,7 +64,7 @@ function bootstrap_examples() { eval $env_vars fi fi - done < <(find "${EXAMPLES_DIR}" -maxdepth 1 -type d -name "*") + done < <(find "${EXAMPLES_DIR}" -maxdepth 1 -type d) } echo ":: Ensuring all crate versions match..."