Fixed reqs check in scripts. #1841
Conversation
WalkthroughThis change updates the command existence checks in multiple shell scripts by adding double quotes around the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
scripts/vortex/download-db-acquia.sh (1)
59-63: Duplicate command-existence checks – consider DRYing upYou already verify
php,curl, andgunziphere. A second standalonecommand -v curlappears at lines 82-84 (unchanged in this PR), resulting in redundant checks and a minor maintenance smell.-# Pre-flight checks. -command -v curl >/dev/null || (fail "curl command is not available." && exit 1) +# Pre-flight checks handled above (php curl gunzip loop)Removing the duplicate keeps the script concise while preserving behaviour.
scripts/vortex/task-copy-db-acquia.sh (1)
74-80: Addphpto the pre-flight command listThe script invokes PHP in both
extract_json_last_value()andextract_json_value()yet the pre-flight loop currently checks only forcurl. When PHP is missing the script will fail later at runtime instead of up-front.-#shellcheck disable=SC2043 -for cmd in curl; do command -v "${cmd}" >/dev/null || { +#shellcheck disable=SC2043 +for cmd in php curl; do command -v "${cmd}" >/dev/null || {Adding the extra check keeps the behaviour consistent with other Vortex scripts and surfaces the problem early.
♻️ Duplicate comments (8)
scripts/vortex/login-container-registry.sh (1)
47-51: Identical hardening already coveredSame quoting hardening as noted in
deploy-container-registry.sh; feedback applies here too.scripts/vortex/reset.sh (1)
22-26: Identical hardening already coveredSame quoting hardening as noted earlier; feedback applies here too.
scripts/vortex/notify-newrelic.sh (1)
56-60: Identical hardening already coveredSame quoting hardening as noted earlier; feedback applies here too.
scripts/vortex/update-vortex.sh (1)
44-47: Identical hardening already coveredSame quoting hardening as noted earlier; feedback applies here too.
scripts/vortex/task-copy-files-acquia.sh (1)
53-57: Comment duplicated – see earlier note aboutcommand -v --Exactly the same concern/suggestion applies here.
scripts/vortex/notify-github.sh (1)
52-55: Comment duplicated – see earlier note aboutcommand -v --Same recommendation as in
doctor.sh.scripts/vortex/notify-webhook.sh (1)
48-51: Comment duplicated – see earlier note aboutcommand -v --Same recommendation as in
doctor.sh.scripts/vortex/export-db-image.sh (1)
40-43: Comment duplicated – see earlier note aboutcommand -v --Same recommendation as in
doctor.sh.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
scripts/vortex/deploy-container-registry.sh(1 hunks)scripts/vortex/deploy-lagoon.sh(1 hunks)scripts/vortex/deploy-webhook.sh(1 hunks)scripts/vortex/doctor.sh(1 hunks)scripts/vortex/download-db-acquia.sh(1 hunks)scripts/vortex/download-db-container-registry.sh(1 hunks)scripts/vortex/download-db-ftp.sh(1 hunks)scripts/vortex/download-db-lagoon.sh(1 hunks)scripts/vortex/download-db-url.sh(1 hunks)scripts/vortex/export-db-image.sh(1 hunks)scripts/vortex/export-db.sh(1 hunks)scripts/vortex/login-container-registry.sh(1 hunks)scripts/vortex/notify-github.sh(1 hunks)scripts/vortex/notify-jira.sh(1 hunks)scripts/vortex/notify-newrelic.sh(1 hunks)scripts/vortex/notify-webhook.sh(1 hunks)scripts/vortex/reset.sh(1 hunks)scripts/vortex/task-copy-db-acquia.sh(1 hunks)scripts/vortex/task-copy-files-acquia.sh(1 hunks)scripts/vortex/task-custom-lagoon.sh(1 hunks)scripts/vortex/task-purge-cache-acquia.sh(1 hunks)scripts/vortex/update-vortex.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
🔇 Additional comments (12)
scripts/vortex/deploy-container-registry.sh (1)
45-49: Quote addition prevents word-splitting – solid hardening
command -v "${cmd}"avoids edge-case word splitting/globbing and is shell-portable. No further changes needed.scripts/vortex/download-db-container-registry.sh (1)
38-42: Quoting incommand -vhardens the availability check – looks goodAdding quotes around
${cmd}eliminates any edge-case breakage from word-splitting or globs. No further issues spotted in this hunk.scripts/vortex/deploy-lagoon.sh (1)
123-127: Consistent quoting improves robustness of the prerequisite checkSame positive change as in the other scripts; prevents accidental splitting if a command name were ever to contain spaces (unlikely but correct practice). 👍
scripts/vortex/download-db-lagoon.sh (1)
80-83: Good defensive shell practice appliedThe loop now uses
command -v "${cmd}", closing a subtle shell-safety gap. Implementation is correct.scripts/vortex/export-db.sh (1)
32-35: Availability check quoting fixedQuoting
${cmd}removes a shell-parsing pitfall. Change is sound.scripts/vortex/notify-jira.sh (1)
57-60: Prerequisite detection made shell-safeThe quoted form of
command -vis the right pattern; no other issues in this snippet.scripts/vortex/task-custom-lagoon.sh (1)
90-93: Robustness improved by quoting${cmd}Quoting the loop variable eliminates word-splitting/globbing edge-cases. No further issues spotted in this hunk.
scripts/vortex/download-db-url.sh (1)
35-38: Correct quoting applied in command checkGood catch – the check is now safe against names containing spaces/special chars.
scripts/vortex/download-db-ftp.sh (1)
46-49: Command check hardenedThe quoting fix looks correct and fully resolves the shellcheck SC2046 concern.
scripts/vortex/deploy-webhook.sh (1)
34-37: Safer${cmd}lookupChange is straightforward and correct.
scripts/vortex/task-copy-db-acquia.sh (1)
74-80: Quoting fix is correctThe newly added quotes around
${cmd}are a no-op for simple command names but make the loop safe against word-splitting or wildcards. Nice catch.scripts/vortex/task-purge-cache-acquia.sh (1)
53-57: Quoted variable improves robustness — nothing else to changeThe added quotes around
${cmd}make the existence check immune to accidental globbing/IFS issues. The command list already includes bothphpandcurl, so the loop now meets best-practice expectations.
| for cmd in docker pygmy ahoy; do command -v "${cmd}" >/dev/null || { | ||
| fail "Command ${cmd} is not available" | ||
| exit 1 | ||
| }; done |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer command -v -- for full POSIX-safety
Quoting ${cmd} is the right call. One tiny follow-up is to pass -- so that an unlikely command name starting with - can’t be interpreted as an option by command.
-for cmd in docker pygmy ahoy; do command -v "${cmd}" >/dev/null || {
+for cmd in docker pygmy ahoy; do command -v -- "${cmd}" >/dev/null || {🤖 Prompt for AI Agents
In scripts/vortex/doctor.sh around lines 76 to 79, the command check loop uses
`command -v "${cmd}"` but lacks the `--` option for POSIX safety. Update the
command to `command -v -- "${cmd}"` to ensure that any command names starting
with a dash are not misinterpreted as options, preserving correct behavior and
robustness.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1841 +/- ##
========================================
Coverage 74.09% 74.09%
========================================
Files 84 84
Lines 4830 4830
Branches 35 35
========================================
Hits 3579 3579
Misses 1251 1251 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit