Repository navigation
feat: add talm script - #985
Rashid Alam (7h3-3mp7y-m4n) wants to merge 1 commit into
Conversation
Signed-off-by: 7h3-3mp7y-m4n <[email protected]>
WalkthroughA new shell script, Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ 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 (
|
![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 (7)
scripts/install.sh (7)
5-8: DRY up ANSI color codesThe ANSI escape sequences are repeated across the logging functions. Extract the color codes into variables (e.g.,
COLOR_INFO,COLOR_ERROR) to reduce duplication and simplify future updates.
10-15: Clarify JSON parsing dependenciesYour dependency check is solid for the current implementation. If you decide to switch to a JSON parser like
jqfor robustness, be sure to add it to this list. Otherwise, document the current reliance ongrep/sedand its limitations.
27-31: Verify OS mapping against asset namesYou lowercase
uname -sintoOS, but ensure this matches the naming convention in the Talm releases (e.g., some projects usedarwinvsmacos). Add explicit mappings or document supported platforms in the README.
32-40: Extend architecture supportOnly common architectures (
amd64,arm64,i386) are covered. If Talm publishes for others (e.g.,armv7l,s390x), include those in thiscaseblock or handle unknown values with a descriptive error.
47-53: Ensure cleanup and handle download errorsMove the
trap cleanup EXITimmediately after creating the temp dir to guarantee cleanup on any exit. Additionally, checkcurl’s exit code explicitly to surface download issues with a custom error message rather than lettingset -eabort silently.
54-70: Improve installation directory flexibilityAllow users to override the installation directory via an environment variable (e.g.,
PREFIX,TALM_INSTALL_DIR) or a--prefixflag. This enhances adaptability in CI environments or custom user setups.
72-76: Safely overwrite existing binariesBefore moving the new binary into place, check if an existing
talmbinary exists and prompt the user or back it up (e.g., rename totalm.bak). This avoids accidental overwrites:[ -f "$INSTALL_PATH" ] && mv "$INSTALL_PATH" "$INSTALL_PATH.bak"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/install.sh(1 hunks)
🔇 Additional comments (2)
scripts/install.sh (2)
22-25: Good: graceful handling for missing versionThe script correctly checks for an empty
LATEST_VERSIONand exits with a clear error message, preventing ambiguous failures downstream.
42-44: Validate constructed download URLBefore invoking
curl -fL, you can perform a quick HEAD request to catch 404s early:curl -sfI "$DOWNLOAD_URL" >/dev/null || { error "Release asset not found at $DOWNLOAD_URL"; exit 1; }This ensures you fail fast on mistyped URLs or missing binaries.
| #!/bin/sh | ||
|
|
||
| set -e |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance strict mode and portability
Consider using a more portable shebang and stricter error handling. For example, switch to:
#!/usr/bin/env bash
set -euo pipefail
trap 'error "An unexpected error occurred."; exit 1' ERRThis will catch unset variables and pipeline failures more reliably.
🤖 Prompt for AI Agents
In scripts/install.sh at lines 1 to 3, improve script robustness and portability
by changing the shebang from #!/bin/sh to #!/usr/bin/env bash, and enhance error
handling by adding set -euo pipefail to catch unset variables and pipeline
errors. Also, add a trap for ERR to log a custom error message and exit cleanly
on unexpected errors.
| LATEST_VERSION=$(curl -fsSL "https://api.github.com/repos/cozystack/talm/releases/latest" | \ | ||
| grep '"tag_name":' | head -1 | sed -E 's/.*"tag_name": *"([^"]+)".*/\1/') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Robustly fetch GitHub release version
To guard against API rate limits and future schema changes, consider adding the appropriate Accept header:
curl -fsSL -H 'Accept: application/vnd.github.v3+json' "https://api.github.com/repos/cozystack/talm/releases/latest"Also validate the HTTP status code before parsing, or switch to jq for reliable extraction of tag_name.
🤖 Prompt for AI Agents
In scripts/install.sh around lines 19 to 20, the curl command fetching the
latest GitHub release version lacks the required Accept header and does not
validate the HTTP status code, risking failures due to API rate limits or schema
changes. Modify the curl command to include the header 'Accept:
application/vnd.github.v3+json' and check the HTTP response status before
parsing. Replace the grep and sed parsing with jq to reliably extract the
tag_name field from the JSON response.
|
Hey Rashid Alam (@7h3-3mp7y-m4n) thanks for your contribution! Could you please open another PR to Talm repository? I think this might be more correct plase for storing this script |
|
Thank you Andrei Kvapil (@kvaps) for catching that, I appreciate it! Apologies for the inconvenience caused by raising the PR in the wrong repository |
github-actions
Bot
added
the
do-not-merge/work-in-progress
Issue #718
This PR adds a shell script that automatically installs the latest release of Talm based on the system's OS and architecture.
Summary by CodeRabbit