SH installer script#562
Conversation
|
""" WalkthroughA new Bash-based installer script ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant install.sh
participant Python/pip
participant GitHub/PyPI
User->>install.sh: Run install.sh with options
install.sh->>install.sh: Check prerequisites (Python, pip, curl)
install.sh->>install.sh: Create virtual environment in target dir
install.sh->>GitHub/PyPI: Fetch latest jumpstarter-all version
install.sh->>Python/pip: Install jumpstarter-all in venv
install.sh->>install.sh: Create activation script and symlinks
install.sh->>User: Display post-install instructions
Poem
""" ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
install.sh (3)
55-57: Minor typo in usage text
profile d→profile.-After installation, activate the environment, or add this to your profile d with: +After installation, activate the environment, or add this to your profile with:
280-300: Post-install instructions ignore custom install dirMessages always reference
~/.local/jumpstarter; misleading when-dwas supplied. Either interpolate$INSTALL_DIRor reuse the helper script to avoid drift.Example quick fix:
- source ~/.local/jumpstarter/set + source "$INSTALL_DIR/set"
96-106: ShellCheck SC2155 – retain exit code clarityAssigning and checking
$?on the next line is safe here, yet the warning points to future maintenance risks. Declaring first then assigning is clearer:local response response=$(curl -s -f "$package_url")Same pattern recurs for
versionandcmd_name.docs/source/getting-started/installation/packages.md (1)
49-59: Missing fenced-code language spec triggers markdownlint (MD040)Add a language (e.g.,
text) so structure block renders consistently and lint passes.-``` +```text ~/.local/jumpstarter/ ├── venv/ # Python virtual environment @@ └── set # Environment activation script</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 637d0eb16ffc5f95586c9ec2045ad876d3243de1 and 0d4f8110519048ca2de9cd10431a64c351371d0e. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `docs/source/getting-started/installation/packages.md` (3 hunks) * `install.sh` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (3)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>docs/source/getting-started/installation/packages.md (2)</summary>Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>install.sh (1)</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.</details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/source/getting-started/installation/packages.md</summary> 51-51: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 Shellcheck (0.10.0)</summary> <details> <summary>install.sh</summary> [warning] 96-96: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 104-104: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 124-124: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 125-125: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 126-126: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 179-179: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 185-185: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 270-270: Declare and assign separately to avoid masking return values. (SC2155) </details> </details> </details> <details> <summary>⏰ 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). (11)</summary> * GitHub Check: Redirect rules - jumpstarter-docs * GitHub Check: Header rules - jumpstarter-docs * GitHub Check: Pages changed - jumpstarter-docs * GitHub Check: build * GitHub Check: e2e * GitHub Check: pytest-matrix (ubuntu-24.04, 3.11) * GitHub Check: pytest-matrix (macos-15, 3.12) * GitHub Check: pytest-matrix (macos-15, 3.11) * GitHub Check: pytest-matrix (macos-15, 3.13) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.12) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.13) </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
install.sh (2)
3-4: Hardensetoptions (-u,-o pipefail)
Using onlyset -emisses failures hidden in pipelines or from unset variables. Adopt the standard hard-fail triplet for consistency with the rest of Jumpstarter’s shell scripts.-set -e +set -euo pipefail
212-233:sethelper still hard-codes~/.local/jumpstarter– breaks-dinstalls
The generated helper ignores customINSTALL_DIR, so environments installed to/opt/jumpstarter(or any other path) won’t activate correctly.Inject the actual
$INSTALL_DIRat generation time (or resolve it at runtime):-cat > "$SET_SCRIPT" << 'EOF' +cat > "$SET_SCRIPT" << EOF ... -JUMPSTARTER_DIR="$HOME/.local/jumpstarter" +JUMPSTARTER_DIR="$(printf '%q' "$INSTALL_DIR")" VENV_DIR="$JUMPSTARTER_DIR/venv"
🧹 Nitpick comments (1)
install.sh (1)
246-252: Symlink loop may introduce duplicates; filter existing links
Running the installer twice will continuously overwrite links and could leave stale binaries from an older venv. Consider cleaning the target bin dir first or checkingln -sftargets to avoid doppelgangers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/getting-started/installation/packages.md(3 hunks)install.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
install.sh (1)
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
docs/source/getting-started/installation/packages.md (2)
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
🪛 Shellcheck (0.10.0)
install.sh
[warning] 96-96: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 104-104: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 124-124: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 125-125: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 126-126: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 179-179: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 185-185: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 248-248: Declare and assign separately to avoid masking return values.
(SC2155)
🪛 markdownlint-cli2 (0.17.2)
docs/source/getting-started/installation/packages.md
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (7)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
🔇 Additional comments (1)
docs/source/getting-started/installation/packages.md (1)
51-59: Specify language for fenced code block (MD040)
Markdown-lint flags this section – add a language identifier to improve highlighting and satisfy CI.-``` +```text ~/.local/jumpstarter/ ├── venv/ # Python virtual environment ...[ suggest_nitpick ] </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
install.sh (2)
3-3: Harden the script flags (-u&-o pipefail)
The installer still only usesset -e. Please add-uand-o pipefailfor the usual robustness we enforce in Jumpstarter shell scripts.-set -e +set -euo pipefail
221-234: Activation script still hard-codes~/.local/jumpstarter
Custom-d /opt/jumpstarterinstalls will generate asethelper that points to the wrong directory, breaking PATH and venv detection. Inject$INSTALL_DIRinstead of the fixed path.
🧹 Nitpick comments (2)
install.sh (1)
129-132: Minor: avoid SC2155 in version parsing
Declareversionfirst, then assign, to preserve$?from command substitution (future-proofing if extra checks are added).local version version=$(python3 -c 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")')docs/source/getting-started/installation/packages.md (1)
51-59: Add language hint to fenced tree block (MD040)
The directory tree block is missing a language tag, tripping markdown-lint. Appendtext(orbash) to keep tooling quiet.-``` +```text ~/.local/jumpstarter/ ├── venv/ ...</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 64b5af65b7b0b212d4f21a51f89f7fb6da13cde3 and a8d25c93c8ae52150e5f4efc11da4f4a2bec7cd5. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `docs/source/getting-started/installation/packages.md` (3 hunks) * `install.sh` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (3)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>docs/source/getting-started/installation/packages.md (2)</summary>Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>install.sh (1)</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.</details> </details><details> <summary>🧬 Code Graph Analysis (1)</summary> <details> <summary>install.sh (1)</summary><blockquote> <details> <summary>packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (1)</summary> * `install` (61-110) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/source/getting-started/installation/packages.md</summary> 51-51: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 Shellcheck (0.10.0)</summary> <details> <summary>install.sh</summary> [warning] 101-101: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 109-109: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 129-129: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 130-130: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 131-131: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 184-184: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 190-190: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 253-253: Declare and assign separately to avoid masking return values. (SC2155) </details> </details> </details> <details> <summary>⏰ 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). (10)</summary> * GitHub Check: Redirect rules - jumpstarter-docs * GitHub Check: Header rules - jumpstarter-docs * GitHub Check: Pages changed - jumpstarter-docs * GitHub Check: e2e * GitHub Check: pytest-matrix (macos-15, 3.13) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.13) * GitHub Check: pytest-matrix (macos-15, 3.12) * GitHub Check: pytest-matrix (macos-15, 3.11) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.12) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.11) </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
install.sh (3)
3-3: Harden the script with-uand-o pipefail
Using onlyset -emisses failures from unset variables and errors inside pipelines. Adopt the project-wide convention:-set -e +set -euo pipefail
211-218:sethelper still hard-codes default path & uses literal heredoc
The helper ignores-d /custom/pathbecause:
- The heredoc is quoted (
<< 'EOF'), preventing$INSTALL_DIRexpansion.JUMPSTARTER_DIRis forced to$HOME/.local/jumpstarter.Same concern was raised earlier and remains unresolved. Inject the actual
INSTALL_DIR, or derive it at runtime, and drop the quoted heredoc.
257-274: Post-install message disregards-dflag
All paths are hard-coded to~/.local/jumpstarter, so users installing elsewhere copy invalid commands. Replace with$INSTALL_DIR.
🧹 Nitpick comments (3)
install.sh (2)
59-61: Typo in help text – stray “d”
profile dshould beprofileto avoid confusing users.-After installation, activate the environment, or add this to your profile d with: +After installation, activate the environment, or add this to your profile with:
102-108: Fragile HTML scraping for version detection
Parsing PyPI-style HTML withawk | grep | sedis brittle and may break with minor markup changes. Consider:pip index versions jumpstarter-all --index-url "$index_url" \ | awk '/AVAILABLE VERSIONS/ {getline; print $1; exit}'
pipguarantees stable output and avoids HTML coupling.docs/source/getting-started/installation/packages.md (1)
51-59: Add language identifier to fenced block
Markdown-lint flags this block; specify a language (e.g.,text) to silence MD040.-``` +```text ~/.local/jumpstarter/ ├── venv/ ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/getting-started/installation/packages.md(3 hunks)install.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
docs/source/getting-started/installation/packages.md (2)
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
install.sh (1)
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
🪛 markdownlint-cli2 (0.17.2)
docs/source/getting-started/installation/packages.md
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.10.0)
install.sh
[warning] 103-103: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 123-123: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 124-124: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 125-125: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 178-178: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 184-184: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 247-247: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ 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). (8)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
|
|
||
| Activate for current session | ||
| ```{code-block} console | ||
| source ~/.local/jumpstarter/set |
There was a problem hiding this comment.
Or the Custom directory if specified, same for the Uninstalling part.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
install.sh (2)
3-4: Add-uand-o pipefailto hard-fail on unset vars and pipeline errors
set -ealone misses failures in pipelines and silently uses empty variables. Harden the script with the usual trio:-set -e +set -euo pipefail
215-218:sethelper still hard-codes~/.local/jumpstarter; breaks-dinstalls
Previous review pointed this out—custom install dirs still generate a helper with the wrong path. Inject$INSTALL_DIRinstead of$HOME.-cat > "$SET_SCRIPT" << 'EOF' +cat > "$SET_SCRIPT" << EOF ... -JUMPSTARTER_DIR="$HOME/.local/jumpstarter" +JUMPSTARTER_DIR="$(printf '%q' "$INSTALL_DIR")" VENV_DIR="\$JUMPSTARTER_DIR/venv" EOF
🧹 Nitpick comments (2)
install.sh (1)
121-134: Minor: accept future Python ≥ 3.11, not only 3.x
If Python 4 ever ships, the current check (major == 3 && minor >= 11) will reject it.- if [ "$major" -eq 3 ] && [ "$minor" -ge 11 ]; then + if { [ "$major" -eq 3 ] && [ "$minor" -ge 11 ]; } || [ "$major" -gt 3 ]; thendocs/source/getting-started/installation/packages.md (1)
51-59: Missing language tag on fenced block (MD040)
The directory-tree snippet lacks a language spec and triggers markdown-lint.-``` +```text ~/.local/jumpstarter/ ├── venv/ ...</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d7fe0b303e3210da257822b69eff3c1bcd5ff890 and 671e90419c61f7b3b8179399656ecdaa4ffe0686. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `docs/source/getting-started/installation/packages.md` (3 hunks) * `install.sh` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (3)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>docs/source/getting-started/installation/packages.md (2)</summary>Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>install.sh (1)</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.</details> </details><details> <summary>🧬 Code Graph Analysis (1)</summary> <details> <summary>install.sh (1)</summary><blockquote> <details> <summary>packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (1)</summary> * `install` (61-110) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/source/getting-started/installation/packages.md</summary> 51-51: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 Shellcheck (0.10.0)</summary> <details> <summary>install.sh</summary> [warning] 103-103: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 123-123: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 124-124: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 125-125: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 178-178: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 184-184: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 247-247: Declare and assign separately to avoid masking return values. (SC2155) </details> </details> </details> <details> <summary>⏰ 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). (10)</summary> * GitHub Check: Redirect rules - jumpstarter-docs * GitHub Check: Header rules - jumpstarter-docs * GitHub Check: Pages changed - jumpstarter-docs * GitHub Check: pytest-matrix (ubuntu-24.04, 3.13) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.11) * GitHub Check: pytest-matrix (macos-15, 3.12) * GitHub Check: pytest-matrix (macos-15, 3.11) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.12) * GitHub Check: pytest-matrix (macos-15, 3.13) * GitHub Check: e2e </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| SOURCE="${SOURCE:-$DEFAULT_SOURCE}" | ||
|
|
||
| # Main installation process | ||
| main() { |
There was a problem hiding this comment.
a trap to do a cleanup in case of failure would be useful?
There was a problem hiding this comment.
ok, I left this for a follow up if that´s ok
|
ah wait benny, I asked for re-review but I didn't see your comments, taking care of them, thanks! |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
install.sh (2)
2-2: Extend safety flags toset -euo pipefail
Onlyset -eis enabled; unset variables and failures inside pipelines will be silently ignored. Align with project conventions and harden the script:-set -e +set -euo pipefail
203-224: Hard-coded$HOME/.local/jumpstarterbreaks-dinstallsThe generated
sethelper ignores the user-selected$INSTALL_DIR, forcing every shell to look in~/.local/jumpstarter. A custom install to/opt/jumpstarter(or any other path) renders the helper unusable.Inject the real
INSTALL_DIRat generation time:-cat > "${SET_SCRIPT}" << 'EOF' +INSTALL_DIR_ESCAPED="$(printf '%q' "${INSTALL_DIR}")" +# NOTE: un-quoted EOF to allow variable interpolation +cat > "${SET_SCRIPT}" << EOF @@ -JUMPSTARTER_DIR="$HOME/.local/jumpstarter" +JUMPSTARTER_DIR="${INSTALL_DIR_ESCAPED}" VENV_DIR="\$JUMPSTARTER_DIR/venv"This keeps the helper consistent with the actual installation location.
🧹 Nitpick comments (2)
install.sh (1)
90-108: Masking exit status with inlinelocal latest_version=$(…)ShellCheck (SC2155) warns that declaring and assigning on the same line hides the exit status of the preceding pipeline, which may lead to false positives in error handling. Split declaration and assignment:
- local latest_version=$(echo "${output}" | grep "^jumpstarter-all (" | sed 's/jumpstarter-all (\([^)]*\)).*/\1/') + local latest_version + latest_version=$(echo "${output}" | grep "^jumpstarter-all (" | sed 's/jumpstarter-all (\([^)]*\)).*/\1/')docs/source/getting-started/installation/packages.md (1)
51-58: Add language identifier to fenced code blockMarkdown-lint flags this block (MD040). Specify a language to improve syntax highlighting and tooling compliance:
-``` +```text ~/.local/jumpstarter/ ├── venv/ # Python virtual environment ...</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 671e90419c61f7b3b8179399656ecdaa4ffe0686 and 025bbc40960358128b57e35257dda3cd2f1fb13e. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `docs/source/getting-started/installation/packages.md` (3 hunks) * `install.sh` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (3)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>docs/source/getting-started/installation/packages.md (2)</summary>Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>install.sh (2)</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/source/getting-started/installation/packages.md</summary> 51-51: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 Shellcheck (0.10.0)</summary> <details> <summary>install.sh</summary> [warning] 102-102: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 117-117: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 118-118: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 119-119: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 172-172: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 178-178: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 241-241: Declare and assign separately to avoid masking return values. (SC2155) </details> </details> </details> <details> <summary>⏰ 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). (10)</summary> * GitHub Check: e2e * GitHub Check: pytest-matrix (macos-15, 3.12) * GitHub Check: pytest-matrix (macos-15, 3.13) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.13) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.11) * GitHub Check: pytest-matrix (macos-15, 3.11) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.12) * GitHub Check: Redirect rules - jumpstarter-docs * GitHub Check: Header rules - jumpstarter-docs * GitHub Check: Pages changed - jumpstarter-docs </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
install.sh (1)
2-2: Harden the script withset -euo pipefail
The project’s shell-script convention (see learning #339) is still not applied;set -ealone ignores errors in pipelines and unset vars.-set -e +set -euo pipefail
🧹 Nitpick comments (4)
install.sh (2)
205-219: Generatedsethelper misses safety flags
For consistency with the main installer and to avoid silent errors inside the helper, prependset -euo pipefailto the generated script.#!/bin/bash +# Fail on error, undefined vars, or pipeline errors +set -euo pipefail # Jumpstarter environment activation script
234-239: Quote path expansion in the symlink loop for edge-case safety
File names containing spaces or glob metacharacters would break the loop. A small quoting tweak eliminates that risk:- for cmd in "${VENV_DIR}/bin"/j*; do - if [ -f "${cmd}" ]; then - local cmd_name=$(basename "${cmd}") - ln -sf "${cmd}" "${bin_dir}/${cmd_name}" + for cmd in "${VENV_DIR}/bin"/j*; do + if [ -f "${cmd}" ]; then + cmd_name="$(basename "$cmd")" + ln -sf "$cmd" "${bin_dir}/${cmd_name}" fi donedocs/source/getting-started/installation/packages.md (2)
51-58: Specify a language for the fenced code block to satisfy MD040
Markdown-lint flags this block. Usetext(ortree) so tooling and rendering stay happy:-``` +```text ~/.local/jumpstarter/ ├── venv/ # Python virtual environment ...--- `62-70`: **Document custom install path with a placeholder** Hard-coding `~/.local/jumpstarter` risks users copy-pasting the wrong path after a `-d` install. Consider a placeholder to make the docs self-consistent with the script: ```diff -source ~/.local/jumpstarter/set +source <INSTALL_DIR>/set # replace <INSTALL_DIR> with your chosen directory
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/getting-started/installation/packages.md(3 hunks)install.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
docs/source/getting-started/installation/packages.md (2)
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
install.sh (2)
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
🪛 markdownlint-cli2 (0.17.2)
docs/source/getting-started/installation/packages.md
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.10.0)
install.sh
[warning] 102-102: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 117-117: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 118-118: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 119-119: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 172-172: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 178-178: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 236-236: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ 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). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: e2e
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
install.sh (1)
2-2: Add-uand-o pipefailto the safety flags (again)
We previously asked forset -euo pipefail; the script still uses onlyset -e.
Adding the two extra options protects against unset variables and errors swallowed in pipelines and is the house style for Jumpstarter shell scripts.
🧹 Nitpick comments (3)
install.sh (2)
115-121: Split assignment from command to keep the real exit status
ShellCheck SC2155 is right here: combining thepython3 -ccall with the
assignment masks the command’s exit status (and makes debugging harder).
Same pattern recurs later forversion,major,minor, and a few other
spots.- local version=$(python3 -c "import sys; print(f'{sys.version_info.major}.{sys.version_info.minor}')") + local version + if ! version="$(python3 -c 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")')" ; then + print_error "Failed to obtain Python version" + return 1 + fiRepeat for
major,minor,index_url,version(lines 172-179) and the
symlink loop (line 236).Also applies to: 117-119
205-219: Quote embedded paths to tolerate spaces in$INSTALL_DIR
Inside the generatedsethelper the${INSTALL_DIR}/binfragments are not quoted.
If a user chooses a directory containing spaces, PATH manipulation will break.-if [[ ":\$PATH:" != *":${INSTALL_DIR}/bin:"* ]]; then - export PATH="${INSTALL_DIR}/bin:\$PATH" +if [[ ":\$PATH:" != *":${INSTALL_DIR}/bin:"* ]]; then + export PATH="${INSTALL_DIR}/bin:\$PATH" fiSurround the expansion (
"${INSTALL_DIR}/bin") with double quotes in both
places.docs/source/getting-started/installation/packages.md (1)
51-58: Specify a language for the fenced code block
Markdown-lint MD040 is complaining. Addtext(orconsole) after the
backticks so the tree renders with consistent styling.``` <!-- current --> ```text <!-- suggested -->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/getting-started/installation/packages.md(3 hunks)install.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
docs/source/getting-started/installation/packages.md (2)
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
install.sh (2)
Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer using `set -e` at the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like `|| exit 1` for individual commands.
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
🧬 Code Graph Analysis (1)
install.sh (1)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (1)
install(61-110)
🪛 markdownlint-cli2 (0.17.2)
docs/source/getting-started/installation/packages.md
51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.10.0)
install.sh
[warning] 102-102: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 117-117: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 118-118: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 119-119: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 172-172: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 178-178: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 236-236: Declare and assign separately to avoid masking return values.
(SC2155)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
install.sh (1)
2-2: Harden script withset -euo pipefailThe script still uses only
set -e. This misses failures caused by unset variables (-u) or errors hidden in pipelines (-o pipefail). A prior review already pointed this out; please incorporate the change:-set -e +set -euo pipefail
🧹 Nitpick comments (4)
install.sh (2)
205-219: Here-doc loses safety for paths with spaces / special chars
cat > "${SET_SCRIPT}" << EOFexpands${INSTALL_DIR}inside the here-doc, injecting a raw path into the generated script.
IfINSTALL_DIRcontains spaces,"or$, the resulting script may break.Safest pattern:
printf '%q\n' "${INSTALL_DIR}" > /tmp/install_dir_escaped INSTALL_DIR_ESCAPED=$(cat /tmp/install_dir_escaped) cat > "$SET_SCRIPT" <<EOF #!/bin/bash # Jumpstarter environment activation script JUMPSTARTER_DIR=${INSTALL_DIR_ESCAPED} VENV_DIR="\$JUMPSTARTER_DIR/venv" … EOFThis preserves correctness without complex quoting rules.
234-239:for cmd in "${VENV_DIR}/bin"/j*breaks when the path contains spacesBecause the glob sits outside the quotes, word-splitting occurs if the venv path has spaces. Safer:
find "${VENV_DIR}/bin" -maxdepth 1 -type f -name 'j*' -print0 | while IFS= read -r -d '' cmd; do ln -sf "$cmd" "${bin_dir}/$(basename "$cmd")" donedocs/source/getting-started/installation/packages.md (2)
51-58: Specify a language for the fenced blockMarkdown-lint flags this block (
MD040).
Addconsolefor consistency with the other blocks:-``` +```console ~/.local/jumpstarter/ ├── venv/ # Python virtual environment …
62-70: Replace hard-coded path with placeholder to match custom-dinstallsThe activation examples still show a literal
~/.local/jumpstarter.
Readers who used-d /opt/jumpstarterwill copy the wrong command.Use an env placeholder or mention substitution explicitly, e.g.:
```console source <INSTALL_DIR>/set # replace <INSTALL_DIR> with your chosen directoryand same for the profile snippet. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 14cc140de2f5389bd768bdb47b29e25c2993da48 and b159d3328424d0083f1b374008d91542d87b2647. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `docs/source/getting-started/installation/packages.md` (3 hunks) * `install.sh` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (3)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>docs/source/getting-started/installation/packages.md (5)</summary>Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#425
File: packages/jumpstarter-driver-tasmota/README.md:23-23
Timestamp: 2025-05-07T18:33:56.914Z
Learning: In Jumpstarter documentation, configuration parameters should be presented in markdown tables rather than bulleted lists for better readability.Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#200
File: contrib/drivers/shell/jumpstarter_driver_shell/driver.py:52-63
Timestamp: 2025-01-16T22:26:03.037Z
Learning: Functions prefixed with underscore (e.g.,_run_inline_shell_script) are internal implementation details and don't require extensive public API documentation.Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#483
File: docs/multiversion.sh:4-5
Timestamp: 2025-05-13T18:08:55.296Z
Learning: The scriptdocs/multiversion.shis intentionally repository-specific and not meant to be generic. The hardcoded branches ("main", "release-0.5", "release-0.6") are deliberate, and suggestions to make the script more generic by dynamically detecting branches should be avoided.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> <details> <summary>install.sh (2)</summary>Learnt from: mangelajo
PR: jumpstarter-dev/jumpstarter#339
File: packages/jumpstarter-driver-flashers/oci_bundles/build_bundle.sh:12-12
Timestamp: 2025-03-12T16:55:13.031Z
Learning: For shell scripts in the Jumpstarter project, prefer usingset -eat the beginning of scripts to handle errors consistently throughout, rather than adding explicit error checks like|| exit 1for individual commands.Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).</details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/source/getting-started/installation/packages.md</summary> 51-51: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 Shellcheck (0.10.0)</summary> <details> <summary>install.sh</summary> [warning] 102-102: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 117-117: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 118-118: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 119-119: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 172-172: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 178-178: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 236-236: Declare and assign separately to avoid masking return values. (SC2155) </details> </details> </details> <details> <summary>⏰ 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). (10)</summary> * GitHub Check: Redirect rules - jumpstarter-docs * GitHub Check: Header rules - jumpstarter-docs * GitHub Check: Pages changed - jumpstarter-docs * GitHub Check: pytest-matrix (macos-15, 3.11) * GitHub Check: pytest-matrix (macos-15, 3.12) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.13) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.12) * GitHub Check: pytest-matrix (macos-15, 3.13) * GitHub Check: pytest-matrix (ubuntu-24.04, 3.11) * GitHub Check: e2e </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>install.sh (1)</summary> `94-109`: **`pip index versions` may be unavailable → add capability check / fallback** `pip index versions` was introduced in pip 21. If the host has an older pip, this call will fail and the installer aborts before it even upgrades pip inside the venv. Consider: 1. Verifying the local pip supports the sub-command, or 2. Falling back to `curl`/`grep` against the simple-index HTML. Example patch (capability probe): ```diff - if ! output="$(python3 -m pip index versions --index-url "${index_url}" --pre jumpstarter-all 2>/dev/null)"; then + if ! python3 -m pip help index >/dev/null 2>&1; then + print_warning "'pip index' not available, using HTML scrape fallback" + if ! output="$(curl -fsSL "${index_url}/jumpstarter-all/" | grep -Eo '>[0-9]+\.[0-9]+\.[^<]+' )"; then + print_error "Failed to obtain package versions from ${index_url}" + return 1 + fi + elif ! output="$(python3 -m pip index versions --index-url "${index_url}" --pre jumpstarter-all 2>/dev/null)"; then print_error "Failed to get jumpstarter-all versions from ${index_url}" return 1 fi
| print_info "Creating virtual environment in ${VENV_DIR}" | ||
|
|
||
| if [ -d "${VENV_DIR}" ]; then | ||
| print_warning "Virtual environment already exists, removing it" | ||
| rm -rf "${VENV_DIR}" | ||
| fi | ||
|
|
||
| python3 -m venv "${VENV_DIR}" | ||
| print_success "Virtual environment created" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unconditional rm -rf risks accidental data loss
create_venv() deletes an existing virtual-env directory without prompting.
If a user reruns the installer in the same prefix, this can wipe other manually-installed packages or local data lying in the venv.
At minimum, print a confirmation message (or require --force) before removal, or move the old venv aside for inspection.
🤖 Prompt for AI Agents
In install.sh around lines 158 to 166, the script unconditionally deletes the
existing virtual environment directory with rm -rf, risking accidental data
loss. Modify the script to either prompt the user for confirmation before
removing the existing venv directory or require a --force flag to proceed with
deletion. Alternatively, implement logic to move the old venv directory aside
(e.g., rename or backup) instead of deleting it outright, allowing inspection
and preventing loss of manually installed packages or local data.
3 participants
Summary by CodeRabbit
New Features
Documentation