-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
style: unify Bash code style #5568
base: dev
Are you sure you want to change the base?
Conversation
89d8fa2
to
993558a
Compare
@@ -1,5 +1,4 @@ | |||
#!/bin/bash | |||
DIR=$(dirname "$0") | |||
|
|||
|
|||
DIR=$(pwd "$0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Immediately overwritten in line 4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
echo -e "${RED}Error: $DB_NAME database does not exist!${RESET}" | ||
echo "To create it, get the .env file from LastPass then run:" | ||
echo 'To create it, make sure you have the environment variables from Bitwarden and run:' | ||
echo | ||
echo -e " ${BOLD}yarn workspace @tupaia/database setup-test-database${RESET}" | ||
echo | ||
echo -e "If you’re missing environment variables, see ${MAGENTA}https://beyond-essential.slab.com/posts/tupaia-monorepo-setup-v5egpdpq#hvfnz-set-environment-variables${RESET}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
07069e7
to
d794119
Compare
remove unused var
portable shebang in `mergeEnvForDB.sh`
cf05c60
to
1e92207
Compare
@@ -49,7 +49,6 @@ for dependency in ${internal_dependencies[@]}; do | |||
done | |||
|
|||
# remove any duplicates | |||
deduplicated_union=() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never used and not exported
e96bc0a
to
b68f4df
Compare
7e25192
to
1ed8e57
Compare
if `cd` fails in this script, then the script’s behaviour can become unpredictable
# Conflicts: # packages/data-api/scripts/installMvRefreshModule.sh # packages/data-lake-api/scripts/checkTestDataLakeExists.sh # packages/database/scripts/checkTestDatabaseExists.sh # packages/database/scripts/dumpDatabase.sh # packages/database/scripts/setupTestDatabase.sh # scripts/bash/downloadEnvironmentVariables.sh # scripts/bash/getPackagesWithEnvFiles.sh
Took some time over the weekend to read up on Bash. This ended up being a logical place for me to apply some of those learnings. Completely understandable if no one wants to review this, especially considering it touches some deployment scripts.
Tip
This is one to review commit-by-commit.
I’ve only made changes to shell scripts specifically targeting Bash. Plain
sh
scripts are untouched. Changes are largely informed by ShellCheck, shellharden and BashPitfalls.Changes
Note
Other than prettifying a few outputs to the terminal, this is a pretty pure refactor with no material/logical/algorithmic changes.
#!/usr/bin/env bash
over#!/bin/bash
.-l
is set. I’m not sure why we use-l
in shell scripts, and I don’t believeset -l
is a valid option, so leaving these untouched.[[ ... ]]
.${...}
when expanding variables unless meaningfully more readable (or necessary, like in"${foo}bar"
).$(...)
over`...`
for command substitutions.[[ ... ]]
over[ ... ]
for tests.(( ... ))
over[[ ... ]]
for arithmetic comparisons.=
over==
in tests.+=
syntax for concatenation of strings and arrays..
oversource
command.source
eminently more readable, but.
is more portable. However, in Bash, they are equivalent, so there is a real argument for us to prefersource
in Bash scripts. ❗ Open to opinions!ansiControlSequences.sh
for convenience variables like${RED}
and${RESET}
for prettifying stdout. This honours theNO_COLOR
environment variable if the user has it set.download-env-vars
,backendStartDev.sh
anddumpDatabase.sh
,patchMvRefereshModule.sh
.(Sorry about all the force pushes. I squashed a lot of commits to try and keep the commit history clean 😅)