docs: update install instructions #72
Conversation
WalkthroughThis update introduces a new Changes
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
ascandone
force-pushed
the
numscript-cli-installer
branch
from
09b53ea to
e5c1b23
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
==========================================
- Coverage 72.47% 72.28% -0.20%
==========================================
Files 36 37 +1
Lines 4080 4091 +11
==========================================
Hits 2957 2957
- Misses 995 1006 +11
Partials 128 128 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
README.md (1)
20-24: Heading level & sub-headings violate Markdown lint, use proper h2/h3 hierarchy
### Installationjumps from the top-level#directly to h3, and the subsequent Using … lines abuse emphasis in place of headings.
Switch to an h2 followed by h3 sub-headings to satisfy MD001 / MD036 and improve readability.-### Installation - -You can install the `numscript` cli with one of the following ways: - -**Using curl** +## Installation + +You can install the `numscript` CLI using one of the following methods: + +### Using curlinstall.sh (1)
6-23: Addset -uandtrapfor safer bash script
set -ealone doesn’t guard against unset vars and makes debugging harder.
Recommend:-set -e +set -euo pipefail +trap 'echo "❌ Installation failed"; exit 1' ERR
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.goreleaser.yamlis excluded by!**/*.yaml
📒 Files selected for processing (2)
README.md(1 hunks)install.sh(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
20-20: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
24-24: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
30-30: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
install.sh (1)
42-43: Asset name still contains an extra dot – download will 404
TheFILENAMEpattern wasn’t fixed; the unwanted dot afternumscript_remains, so the generated URL does not match the published assets.-FILENAME="numscript_.${LATEST_TAG#v}_${PLATFORM}_${ARCH}.${EXT}" +FILENAME="numscript_${LATEST_TAG#v}_${PLATFORM}_${ARCH}.${EXT}"
🧹 Nitpick comments (2)
install.sh (2)
1-3: Harden script withset -euo pipefail
Enabling-uand-o pipefailprevents silent failures due to unset vars or broken pipes.-set -e +set -euo pipefail
30-32: Tag detection via redirect is brittle
Parsing theLocationheader can break if GitHub changes its redirect format or adds query params. The releases API is more stable:LATEST_TAG=$(curl -s https://api.github.com/repos/formancehq/numscript/releases/latest | jq -r .tag_name)Requires
jq, but greatly increases reliability.
| INSTALL_PATH="$HOME/.local/bin" | ||
| mkdir -p "$INSTALL_PATH" | ||
|
|
||
| mv "$BIN" "$INSTALL_PATH" | ||
| chmod +x "$INSTALL_PATH/$BIN" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Installation path & chmod are not Windows-friendly
For the Windows branch the script:
- Installs to
$HOME/.local/bin, a Unix-style path that is rarely onPATHin Git-Bash/PowerShell. - Runs
chmod, which is a no-op (or errors) on native Windows.
Consider computing a platform-specific default such as:
if [ "$PLATFORM" = "Windows" ]; then
INSTALL_PATH="$USERPROFILE/bin"
mkdir -p "$INSTALL_PATH"
mv "$BIN" "$INSTALL_PATH"
else
INSTALL_PATH="$HOME/.local/bin"
mkdir -p "$INSTALL_PATH"
mv "$BIN" "$INSTALL_PATH"
chmod +x "$INSTALL_PATH/$BIN"
fi🤖 Prompt for AI Agents
In install.sh around lines 58 to 63, the script uses a Unix-style installation
path and runs chmod, which are not appropriate for Windows. Modify the script to
detect if PLATFORM is Windows; if so, set INSTALL_PATH to "$USERPROFILE/bin",
create the directory, and move the binary there without running chmod. For other
platforms, keep the existing behavior of installing to "$HOME/.local/bin",
creating the directory, moving the binary, and running chmod to make it
executable.
| tar -xf "$FILENAME" numscript | ||
| BIN="numscript" | ||
| fi |
There was a problem hiding this comment.
Extraction will fail: missing -z flag for .tar.gz archives
tar -xf cannot decompress gzip streams; the command will exit with “gzip: stdin: not in gzip format”.
- tar -xf "$FILENAME" numscript
+ tar -xzf "$FILENAME" numscript📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tar -xf "$FILENAME" numscript | |
| BIN="numscript" | |
| fi | |
| tar -xzf "$FILENAME" numscript | |
| BIN="numscript" | |
| fi |
🤖 Prompt for AI Agents
In install.sh around lines 53 to 55, the tar extraction command is missing the
-z flag needed for decompressing .tar.gz files. Update the tar command to
include the -z option (tar -xzf) to properly handle gzip compressed archives and
avoid extraction errors.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| fi | ||
|
|
||
| # Build file name and URL | ||
| FILENAME="numscript_.${LATEST_TAG#v}_${PLATFORM}_${ARCH}.${EXT}" |
ascandone
force-pushed
the
numscript-cli-installer
branch
from
66b05b9 to
5686c91
Compare
There was a problem hiding this comment.
Bug: Script Filename Error Causes Download Failures
The install.sh script fails to download release assets due to an incorrect filename construction. The script's generated filename (line 42) has two issues: it includes an extra dot after numscript_ and incorrectly strips the 'v' prefix from the version tag. This mismatch with Goreleaser's asset naming convention causes download attempts to fail with a 404 error.
install.sh#L41-L42
Lines 41 to 42 in 5686c91
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
20-21: Fix Markdown heading level – use##instead of###The document jumps from
#to###, violating MD001 and harming screen-reader outline.-### Installation +## Installation
24-34: Promote bold labels to proper sub-headingsUse heading syntax for the two installation methods; this improves navigation and satisfies MD036.
-**Using curl** +### Using curl … -**Using golang toolchain** +### Using Go toolchain
27-27: Consider pinning the install script to a tag for reproducibilityPulling from
mainmeans an arbitrary future commit is executed withbash.
Pointing to a versioned tag (e.g.,v0.0.18) or asking users to review the script first would reduce supply-chain risk.No change required if you accept the trade-off, but worth documenting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.goreleaser.yamlis excluded by!**/*.yaml
📒 Files selected for processing (2)
README.md(1 hunks)install.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- install.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ascandone
PR: formancehq/numscript#72
File: install.sh:28-44
Timestamp: 2025-07-11T08:02:26.428Z
Learning: In the formancehq/numscript repository, release assets are named with an extra dot after the underscore, following the pattern "numscript_.{version}_{platform}_{arch}.{ext}" (e.g., "numscript_.0.0.18_Darwin_arm64.tar.gz"), not "numscript_{version}_{platform}_{arch}.{ext}".
README.md (2)
Learnt from: ascandone
PR: formancehq/numscript#72
File: install.sh:28-44
Timestamp: 2025-07-11T08:02:26.428Z
Learning: In the formancehq/numscript repository, release assets are named with an extra dot after the underscore, following the pattern "numscript_.{version}_{platform}_{arch}.{ext}" (e.g., "numscript_.0.0.18_Darwin_arm64.tar.gz"), not "numscript_{version}_{platform}_{arch}.{ext}".
Learnt from: ascandone
PR: formancehq/numscript#40
File: internal/lsp/codeactions_test.go:177-189
Timestamp: 2025-03-07T12:40:05.628Z
Learning: When reviewing code, focus on substantive issues related to core functionality rather than minor details in test utilities. For ascandone specifically, avoid nitpicky comments about testing utilities.
🪛 markdownlint-cli2 (0.17.2)
README.md
20-20: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
24-24: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
30-30: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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). (3)
- GitHub Check: Cursor BugBot
- GitHub Check: Tests
- GitHub Check: Dirty
ascandone
changed the title
I changed the following:
internal/numscript/numscript.go(the main file) tocmd/numscript/main.go, so that it can be installed withgo installinstall.shthat fetches the last released version and installs it