-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
Add shellcheck support and robustify present shell scripts #5294
base: develop
Are you sure you want to change the base?
Changes from all commits
ac438ef
91976cb
beed5fd
b477d0e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
--- | ||
name: Shellcheck | ||
|
||
on: | ||
push: | ||
branches: [develop] | ||
pull_request: | ||
branches: [develop] | ||
|
||
permissions: | ||
contents: read | ||
|
||
jobs: | ||
shellcheck: | ||
name: Check shell scripts | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v4 | ||
- name: Install dependencies | ||
run: | | ||
sudo apt update && sudo apt install -y shellcheck | ||
- name: shellcheck | ||
run: | | ||
git grep -l '^#\( *shellcheck \|!\(/bin/\|/usr/bin/env \)\(sh\|bash\|dash\|ksh\)\)' | xargs shellcheck | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
#!/bin/sh | ||
# shellcheck disable=SC1091 | ||
. "$(dirname "$0")/_/husky.sh" | ||
|
||
pnpm run pre-commit |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
#!/bin/sh | ||
source /root/.shrc | ||
|
||
# shellcheck disable=SC1091 | ||
. /root/.shrc | ||
exec "$@" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,14 +8,20 @@ underline() { ansi 4 "$@"; } | |
# strikethrough() { ansi 9 "$@"; } | ||
red() { ansi 31 "$@"; } | ||
|
||
name=$(basename $0) | ||
name=$(basename "$0") | ||
command=$1 | ||
# TODO: fix this one to become spaces tollerant etc | ||
# If to be used as an array, make it | ||
# args=( "${@:2}" ) | ||
# If to concatenated into a single string and thus loosing ability to pass args with spaces | ||
# args="${*:2}" | ||
# shellcheck disable=SC2124 | ||
args=${@:2} | ||
Comment on lines
+13
to
19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. todo(blocking): I'm fairly certain quoting Do you think it makes sense to change it to something like: diff --git a/run b/run
index f01302c43..ecb392e58 100755
--- a/run
+++ b/run
@@ -10,18 +10,12 @@ red() { ansi 31 "$@"; }
name=$(basename "$0")
command=$1
-# TODO: fix this one to become spaces tollerant etc
-# If to be used as an array, make it
-# args=( "${@:2}" )
-# If to concatenated into a single string and thus loosing ability to pass args with spaces
-# args="${*:2}"
-# shellcheck disable=SC2124
-args=${@:2}
+args=( "${@:2}" )
case $command in
build)
-docker compose build "$args"
+docker compose build "${args[@]}"
;;
sh)
@@ -29,7 +23,7 @@ $RUN mermaid sh
;;
pnpm)
-$RUN mermaid sh -c "pnpm $args"
+$RUN mermaid pnpm "${args[@]}"
;;
dev)
@@ -41,7 +35,7 @@ $RUN --service-ports mermaid sh -c "pnpm run --filter mermaid docs:dev:docker"
;;
cypress)
-$RUN cypress "$args"
+$RUN cypress "${args[@]}"
;;
help|"") I'll be honest, I don't really use this |
||
|
||
case $command in | ||
|
||
build) | ||
docker compose build $args | ||
docker compose build "$args" | ||
;; | ||
|
||
sh) | ||
|
@@ -35,7 +41,7 @@ $RUN --service-ports mermaid sh -c "pnpm run --filter mermaid docs:dev:docker" | |
;; | ||
|
||
cypress) | ||
$RUN cypress $args | ||
$RUN cypress "$args" | ||
;; | ||
|
||
help|"") | ||
|
@@ -52,41 +58,41 @@ ________________________________________________________________________________ | |
|
||
Development Quick Start Guide: | ||
|
||
$(bold ./$name pnpm install) # Install packages | ||
$(bold ./$name dev) # Launch dev server with examples, open http://localhost:9000 | ||
$(bold ./$name docs:dev) # Launch official website, open http://localhost:3333 | ||
$(bold ./"$name" pnpm install) # Install packages | ||
$(bold ./"$name" dev) # Launch dev server with examples, open http://localhost:9000 | ||
$(bold ./"$name" docs:dev) # Launch official website, open http://localhost:3333 | ||
|
||
$(bold ./$name pnpm vitest) # Run watcher for unit tests | ||
$(bold ./$name cypress) # Run integration tests (after starting dev server) | ||
$(bold ./$name pnpm build) # Prepare it for production | ||
$(bold ./"$name" pnpm vitest) # Run watcher for unit tests | ||
$(bold ./"$name" cypress) # Run integration tests (after starting dev server) | ||
$(bold ./"$name" pnpm build) # Prepare it for production | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. although looks odd, it is the only way to make it robust if script would ever be renamed to include space in the name. Alternatively that check could be skipped indeed. |
||
__________________________________________________________________________________________ | ||
|
||
Commands: | ||
|
||
$(bold ./$name build) # Build image | ||
$(bold ./$name cypress) # Run integration tests | ||
$(bold ./$name dev) # Run dev server with examples, open http://localhost:9000 | ||
$(bold ./$name docs:dev) # For docs contributions, open http://localhost:3333 | ||
$(bold ./$name help) # Show this help | ||
$(bold ./$name pnpm) # Run any 'pnpm' command | ||
$(bold ./$name sh) # Open 'sh' inside docker container for development | ||
$(bold ./"$name" build) # Build image | ||
$(bold ./"$name" cypress) # Run integration tests | ||
$(bold ./"$name" dev) # Run dev server with examples, open http://localhost:9000 | ||
$(bold ./"$name" docs:dev) # For docs contributions, open http://localhost:3333 | ||
$(bold ./"$name" help) # Show this help | ||
$(bold ./"$name" pnpm) # Run any 'pnpm' command | ||
$(bold ./"$name" sh) # Open 'sh' inside docker container for development | ||
__________________________________________________________________________________________ | ||
|
||
Examples of frequently used commands: | ||
|
||
$(bold ./$name pnpm add --filter mermaid) $(underline package) | ||
$(bold ./"$name" pnpm add --filter mermaid) $(underline package) | ||
Add package to mermaid | ||
|
||
$(bold ./$name pnpm -w run lint:fix) | ||
$(bold ./"$name" pnpm -w run lint:fix) | ||
Run prettier and ES lint | ||
|
||
$(bold git diff --name-only develop \| xargs ./$name pnpm prettier --write) | ||
$(bold git diff --name-only develop \| xargs ./"$name" pnpm prettier --write) | ||
Prettify everything you added so far | ||
|
||
$(bold ./$name cypress open --project .) | ||
$(bold ./"$name" cypress open --project .) | ||
Open cypress interactive GUI | ||
|
||
$(bold ./$name cypress run --spec cypress/integration/rendering/)$(underline test.spec.ts) | ||
$(bold ./"$name" cypress run --spec cypress/integration/rendering/)$(underline test.spec.ts) | ||
Run specific test in cypress | ||
|
||
$(bold xhost +local:) | ||
|
@@ -99,7 +105,7 @@ echo -n -e "$usage" | |
;; | ||
|
||
*) | ||
message="$(red Unknown command: $command). See $(bold ./$name help) for available commands." | ||
message="$(red Unknown command: "$command"). See $(bold ./"$name" help) for available commands." | ||
echo -n -e "$message\n" >&2 | ||
$0 help | ||
exit 1 | ||
|
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.
Love this!
Just to tidy up, do you think it makes sense to move this into the
.github/workflows/lint.yml
with the other lint job?You'll have to move the
permissions: contents: read
into thejobs.shellcheck
too.