chore: add permissions windows install script #116
Conversation
|
Warning Rate limit exceeded@matiasdaloia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughThe release workflow introduces centralized versioning using APP_NAME and APP_VERSION environment variables, replacing direct git tag queries across all packaging steps. Installation scripts add non-interactive mode detection to use defaults instead of prompting. The confirm function in common.sh now supports non-interactive execution by reading from /dev/tty or using defaults. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
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. Comment |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
98-99: DMG symlink addition requires error handling.The
ln -scommand creates a symlink without checking if it already exists or if the command succeeds. If a directory namedApplicationsalready exists indmg_contents/, or if permissions prevent symlink creation, the DMG will be created without the symlink, but no error will be raised.Consider adding error handling or using
-fflag to overwrite if needed:- ln -s /Applications dmg_contents/Applications + ln -sf /Applications dmg_contents/Applications || log_warn "Failed to create Applications symlink"However, verify that this symlink behavior aligns with
create-dmgtool expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release.yml(1 hunks)scripts/install-linux.sh(1 hunks)scripts/install-macos.sh(2 hunks)scripts/lib/common.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/install-macos.sh (2)
scripts/lib/common.sh (1)
log_info(15-17)scripts/install.sh (1)
log_info(19-21)
scripts/install-linux.sh (2)
scripts/lib/common.sh (1)
log_info(15-17)scripts/install.sh (1)
log_info(19-21)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (4)
scripts/install-linux.sh (1)
144-148: Non-interactive mode detection is well-placed and clear.This check properly detects piped execution and logs appropriately to stderr. The integration with the updated
confirm()function incommon.shensures user prompts will use defaults without blocking.scripts/lib/common.sh (1)
172-194: Non-interactive mode handling inconfirm()is well-designed.The implementation properly handles three scenarios:
- Non-interactive (piped stdin) → uses default without prompting
- Interactive with
/dev/ttyavailable → reads from terminal, allowing prompts even with redirected stdin- Interactive without
/dev/tty→ gracefully falls back to defaultThe prompt format clearly indicates the default option (
[Y/n]vs[y/N]), and the response normalization at line 193 correctly treats empty input as accepting the default. This integration with the installer scripts' non-interactive detection creates a cohesive flow.scripts/install-macos.sh (2)
238-240: Non-interactive default to Homebrew is a reasonable UX choice.The script properly detects non-interactive execution and defaults to Homebrew installation, which is simpler and more suitable for automated/CI environments. The logging is consistent with the Linux installer. However, ensure this behavior is documented for users who pipe the script but want direct installation.
259-259: Correct use of/dev/ttyfor interactive user input.Redirecting the
readcommand to/dev/ttyensures the script can capture terminal input even when stdin is redirected (e.g., from a pipe). This is the correct idiom and prevents the piped script content from being consumed as user input.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(7 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/release.yml (1)
101-108: DMG creation references will fail due to APP_VERSION issue.The DMG volume naming and artifact path reference
${{ env.APP_VERSION }}(line 108), which is dependent on the critical issue identified above. Once APP_VERSION is fixed, this refactoring looks good—it improves maintainability by centralizing version references.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
43-43: Multiple headings with the same content
(MD024, no-duplicate-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). (2)
- GitHub Check: unit_tests
- GitHub Check: integration-tests
🔇 Additional comments (2)
CHANGELOG.md (2)
38-42: ✓ [0.9.1] entry structure and content are correct.The new version entry for [0.9.1] follows the Keep a Changelog format properly, with correct date (2025-11-04) and a concise summary of the fixes. The placement above [0.9.0] is appropriate. Once the duplicate section is removed, this entry will be valid.
197-197: ✓ Changelog link correctly added.The comparison link for [0.9.1] is properly formatted and points to the correct range (v0.9.0...v0.9.1).
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
fix: prevent installation scripts from hanging when piped from curl fix: macos dmg installer fix: macos dmg installer chore: update changelog fix: app version env variable in release workflow chore: update changelog
matiasdaloia
force-pushed
the
chore/mdaloia/add-execution-permissions-installation-scripts
branch
from
d4d28aa to
dd32ceb
Compare
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
matiasdaloia
deleted the
chore/mdaloia/add-execution-permissions-installation-scripts
branch
Summary by CodeRabbit
New Features
Bug Fixes