Add install and uninstall scripts#4
Merged
Merged
Conversation
Signed-off-by: Winford <winford@object.stream>
Signed-off-by: Winford <winford@object.stream>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds release packaging and build configuration, a self-extracting installer, launcher and uninstaller scripts, updated app metadata and release templates, CI workflow formatting/ShellCheck tweaks, and README installation/uninstall documentation. ChangesRelease Packaging and Installation
sequenceDiagram
participant rebar as rebar.config.script
participant relx as relx
participant build as build_artifacts
participant install_sh as install.sh
rebar->>relx: configure relx release and overlays
rebar->>build: escriptize via provider_hooks
build->>install_sh: post_hooks append tarball into install.sh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rebar.config (1)
1-136:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAddress the erlfmt formatting issues.
The pipeline reports code style issues in this file. Run
rebar3 as check fmtto auto-format.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rebar.config` around lines 1 - 136, Run the erlfmt formatter and commit the updated rebar.config: execute `rebar3 as check fmt` to apply the formatting rules defined by the {erlfmt, [...] } entry in this file, review the modified rebar.config to ensure only whitespace/formatting changes were made, and add/commit those changes so the pipeline no longer reports style issues.
🧹 Nitpick comments (1)
rebar.config (1)
95-95: ⚡ Quick winConsider extracting the version to avoid duplication.
The version
"0.1.0"appears both in therelxrelease definition (line 95) and in thepost_hookstar filename (line 132). If the version changes, both locations must be updated manually, which is error-prone.Consider using rebar3's
{vsn, git}or reading from a single source.Also applies to: 132-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rebar.config` at line 95, The release version string "0.1.0" is duplicated in the {release, {atomvm_spectrometer, "0.1.0"}, ...} tuple and the post_hooks tar filename; extract the version into a single source and reference it from both places (e.g., define a Version variable at the top of rebar.config and use that symbol where the release tuple and the post_hooks tar name are declared, or switch the release to {vsn, git} and build the post_hooks filename from that same vsn), then update {release, {atomvm_spectrometer, ...}} and the post_hooks tar filename to use the single version source.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/code_quality_check.yaml:
- Around line 62-64: The YAML block under the GitHub Actions step has incorrect
indentation for the multiline run: | content; update the two shellcheck commands
that follow the run: | line so they are indented two more spaces (total of 8
spaces) to be nested under run: |, ensuring the shellcheck lines are valid YAML
members of the run block (look for the run: | key and the shellcheck -o
require-variable-braces... lines to adjust).
In `@README.md`:
- Line 64: Fix the typo in the README text: replace "buy" with "by" in the
sentence "the user PATH. If not already present, it can be added permanently buy
adding" so it reads "...can be added permanently by adding"; update that exact
string in README.md.
- Line 85: Replace the incorrect word "than" with "then" in the sentence
fragment "application and user cache files. If sudo was required for
installation than" in README.md so it reads "...cache files. If sudo was
required for installation then..."; simply update that word to correct the
grammar.
- Line 65: Fix the PATH export syntax: replace the incorrect variable expansion
string `"{$PATH}:<prefix>/bin"` with the correct shell expansion using braces
around the variable name, i.e., use `"${PATH}:<prefix>/bin"` in the existing
export PATH line so the shell expands PATH properly when running the `export
PATH="...":...` command.
In `@release/install.in`:
- Line 99: The cp command uses a quoted glob so the shell won't expand "*"
(echo_run cp -fr "${tmp_dir}/*" "${dest_dir}/"), causing copy to fail; change it
to pass the glob unquoted so the shell expands it (e.g. use echo_run cp -fr
"${tmp_dir}"/* "${dest_dir}/") or alternatively copy the whole directory without
a glob (e.g. use cp -fr "${tmp_dir}/." "${dest_dir}/" or switch to rsync) —
update the call referencing tmp_dir, dest_dir and the cp -fr invocation
accordingly.
- Line 5: Replace the incorrect year "206" in the copyright header string
"Copyright 206 Winford (UncleGrumpy) <winford@object.stream>" with "2026" so the
license header reads "Copyright 2026 Winford (UncleGrumpy)
<winford@object.stream>" (update the literal in release/install.in).
- Around line 68-72: The echo_run function currently builds cmd="${*}" and then
executes ${cmd}, which allows word-splitting and breaks paths with spaces;
change it to preserve arguments and quote expansions by using the
positional-parameter form (use "$@" for execution) and stop using an unquoted
${cmd}; also update the diagnostic echo to print a safe, quoted representation
of the arguments (e.g., join/format "$@" or use a quoting-aware printer) so
echo_run (the function and its cmd handling) no longer relies on unquoted ${*}
or ${cmd}.
In `@release/uninstall.in`:
- Line 5: Update the copyright year in the header line that currently reads
"Copyright 206 Winford (UncleGrumpy) <winford@object.stream>" to "Copyright 2026
Winford (UncleGrumpy) <winford@object.stream>" so the year is correct; locate
and edit the top-of-file header in release/uninstall.in to replace "206" with
"2026".
- Around line 68-79: The script uses OS=$(uname -o) and matches "GNU/Linux",
which is non-portable; change the OS detection to use uname -s (i.e., replace
uname -o with uname -s) and update the case labels to match the values uname -s
actually returns (use "Darwin" for macOS and "Linux" for Linux) so the cache_dir
assignment in the case block (cache_dir variable and its readonly assignments)
works cross-platform; keep the existing default branch for unknown OS values.
- Around line 49-53: The echo_run function uses an unquoted expansion of ${cmd},
causing word splitting on paths with spaces; update echo_run (the function and
its use of cmd) to preserve arguments and spacing by avoiding building a single
string and instead printing the command for logging while executing the original
arguments safely (use the positional expansion "${@}" for execution and quote
the printed representation), ensuring the function uses "${@}" to run the
command so each argument with spaces is preserved.
---
Outside diff comments:
In `@rebar.config`:
- Around line 1-136: Run the erlfmt formatter and commit the updated
rebar.config: execute `rebar3 as check fmt` to apply the formatting rules
defined by the {erlfmt, [...] } entry in this file, review the modified
rebar.config to ensure only whitespace/formatting changes were made, and
add/commit those changes so the pipeline no longer reports style issues.
---
Nitpick comments:
In `@rebar.config`:
- Line 95: The release version string "0.1.0" is duplicated in the {release,
{atomvm_spectrometer, "0.1.0"}, ...} tuple and the post_hooks tar filename;
extract the version into a single source and reference it from both places
(e.g., define a Version variable at the top of rebar.config and use that symbol
where the release tuple and the post_hooks tar name are declared, or switch the
release to {vsn, git} and build the post_hooks filename from that same vsn),
then update {release, {atomvm_spectrometer, ...}} and the post_hooks tar
filename to use the single version source.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ddeb77ee-b55f-4fcb-9363-8f479d95aeca
📒 Files selected for processing (9)
.github/workflows/check-formatting.yaml.github/workflows/code_quality_check.yamlREADME.mdrebar.configrelease/install.inrelease/spectrometer.app.src.inrelease/spectrometer.inrelease/uninstall.insrc/spectrometer.app.src
UncleGrumpy
force-pushed
the
doc_update
branch
3 times, most recently
from
d01fe02 to
876496a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rebar.config.script`:
- Around line 10-20: The Version value retrieved into the Version variable is
taken directly from git and later injected into a shell command (referenced
around the Version binding and the injection at lines ~139-141), which allows
shell metacharacter injection; fix by validating/sanitizing Version immediately
after assignment—allow only a safe whitelist (e.g., /^[A-Za-z0-9._-]+$/) and
fall back to a fixed safe string if it fails, and/or avoid interpolating into a
shell command by using a safe API (no shell) or by robustly escaping/quoting the
value before use; update the code that injects Version into the shell (the
release-packaging invocation) to use the sanitized/validated variable or a
non-shell invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a08287cd-535c-4929-8913-2c3aad92c133
📒 Files selected for processing (9)
.github/workflows/code_quality_check.yamlREADME.mdrebar.configrebar.config.scriptrelease/install.inrelease/spectrometer.app.src.inrelease/spectrometer.inrelease/uninstall.insrc/spectrometer.app.src
💤 Files with no reviewable changes (1)
- rebar.config
✅ Files skipped from review due to trivial changes (3)
- src/spectrometer.app.src
- README.md
- release/spectrometer.app.src.in
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/code_quality_check.yaml
- release/install.in
- release/uninstall.in
- release/spectrometer.in
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/spectrometer.app.src (1)
14-21: 💤 Low valueConsider reordering applications to follow OTP convention.
The Erlang/OTP convention typically lists
kernelandstdlibfirst in the applications list, as they are the fundamental runtime applications. While the current order is functionally correct, following this convention improves readability.♻️ Suggested reordering
{applications, [ + kernel, + stdlib, compiler, inets, - kernel, ssl, - stdlib, syntax_tools ]},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/spectrometer.app.src` around lines 14 - 21, The applications list in the release config is out of conventional order; reorder the tuple named applications so that kernel and stdlib appear first (e.g., ensure kernel and stdlib are listed before compiler, inets, ssl, syntax_tools) to follow OTP conventions and improve readability while preserving the same entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/spectrometer.app.src`:
- Around line 14-21: The applications list in the release config is out of
conventional order; reorder the tuple named applications so that kernel and
stdlib appear first (e.g., ensure kernel and stdlib are listed before compiler,
inets, ssl, syntax_tools) to follow OTP conventions and improve readability
while preserving the same entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9505d52c-80b0-42f5-857b-c9b6545addf9
📒 Files selected for processing (9)
.github/workflows/code_quality_check.yamlREADME.mdrebar.configrebar.config.scriptrelease/install.inrelease/spectrometer.app.src.inrelease/spectrometer.inrelease/uninstall.insrc/spectrometer.app.src
💤 Files with no reviewable changes (1)
- rebar.config
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- release/spectrometer.in
- release/spectrometer.app.src.in
- release/uninstall.in
- rebar.config.script
- release/install.in
Signed-off-by: Winford <winford@object.stream>
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds self-extracting executable installer and uninstall script, updates docs on how to install and uninstall the app.