-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Run shellcheck on scripts #214
Conversation
Codecov Report
@@ Coverage Diff @@
## v2 #214 +/- ##
=======================================
Coverage 88.70% 88.70%
=======================================
Files 5 5
Lines 124 124
=======================================
Hits 110 110
Misses 14 14 |
Sounds good to me. Please do. Be careful where it mentions needing to double quote to prevent globbing, etc. There are a few places where I'm intentionally not double-quoting, but if you're able to find a way that makes shellcheck happy and keeps the tests passing, then awesome! |
It is very easy
|
Please also see #215 |
It is harder than I thought! 🥊 Not quoting is used to split string at whitespaces. |
How about making a key this way? function make_key {
tr --squeeze-repeats '\t ' '-' <<<"${*}"
} |
The above 👆 function does not exclude empty string elements of an array 😿 |
... this feel like my private war against unquoted variables 🪖 |
I was trying to go for a pure Bash solution, so I don't have to rely on external tools. |
The shell scripts need to run on GitHub's Windows environments, so any tools that don't exist there aren't going to work. |
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.
This is great! Thanks!
for option in "${additional_options[@]}"; do | ||
composer_options+=("${option}") | ||
done | ||
read -r -a additional_options <<<"${additional_composer_options}" |
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.
Oh, nice! I didn't think about using redirection with read
. I was thinking of it only in terms of user interaction.
Thank you! While working on it I've realized Bash should handle command line arguments through |
Description
Check common problems in our shell code.
@ramsey Should I go on and fix reported problems?
Types of changes
PR checklist