Make generate_fun_data.sh POSIX compliant#2
Merged
Merged
Conversation
|
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 (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughConvert generate_fun_data.sh from Bash to POSIX sh; use mktemp-backed cache with EXIT cleanup; rebuild spectrometer; scan main, latest origin/release-X.Y branch, and selected tags (release vs prerelease filtering); update tag scan loop; use cmp-based copy/backup; extend uart entries; add ShellCheck CI step. ChangesPOSIX migration + tag scanning
Sequence Diagram(s)sequenceDiagram
participant Script as generate_fun_data.sh
participant Git as git
participant Spect as spectrometer
participant FS as filesystem
Script->>Git: fetch remotes & tags
Script->>Git: checkout main / release branch / tag
Script->>Spect: spectrometer update (--tag / branch) writing to OUTPUT_FILE in TMP_CACHE_DIR
Spect->>FS: write generated data to TMP_CACHE_DIR/OUTPUT_FILE
Script->>FS: cmp -s TMP_CACHE_DIR/OUTPUT_FILE priv/supported_functions.data
alt different
Script->>FS: mv priv/supported_functions.data -> .bak timestamped
Script->>FS: cp TMP_CACHE_DIR/OUTPUT_FILE -> priv/supported_functions.data
end
Script->>FS: EXIT trap rm -rf TMP_CACHE_DIR
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 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: 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 `@generate_fun_data.sh`:
- Around line 23-26: TMP_CACHE_DIR is set to a non-existent child of the mktemp
dir and the EXIT trap deletes only that child, leaking the real temp directory;
change the logic to capture the mktemp base dir (e.g. TMP_BASE_DIR="$(mktemp
-d)"), set TMP_CACHE_DIR="${TMP_BASE_DIR}/spectrometer_version_cache", create
the child with mkdir -p "${TMP_CACHE_DIR}", and update the trap to rm -rf
"${TMP_BASE_DIR}" so the actual temp directory is always cleaned up; reference
TMP_CACHE_DIR, TMP_BASE_DIR, mktemp and the EXIT trap when making the change.
🪄 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: 4ece2e23-f4db-4431-9855-bfe43aa2bbb4
📒 Files selected for processing (1)
generate_fun_data.sh
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
generate_fun_data.sh (1)
260-270:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
spectrometer updatefailures inside the loop.With
set -e, a single non-zero exit from"$SPECTROMETER" updateaborts the whole script. One bad tag prevents the remaining tags from being scanned. Thegit checkoutalready demonstrates the error-handling pattern used elsewhere in the script.Suggested fix
while IFS= read -r TAG; do [ -z "$TAG" ] && continue echo "=== Scanning tag: $TAG ===" git checkout "$TAG" --quiet 2>/dev/null || { echo "Warning: Could not checkout $TAG"; continue; } - "$SPECTROMETER" update \ - --atomvm-dir "$ATOMVM_DIR" \ - --tag "$TAG" \ - --cache "$TMP_CACHE_DIR" \ - --output "$OUTPUT_FILE" \ - --force \ - --no-tests + if ! "$SPECTROMETER" update \ + --atomvm-dir "$ATOMVM_DIR" \ + --tag "$TAG" \ + --cache "$TMP_CACHE_DIR" \ + --output "$OUTPUT_FILE" \ + --force \ + --no-tests + then + echo "Warning: spectrometer update failed for $TAG, continuing..." + continue + fi echo "" done < "$TMP_TAG_FILE"🤖 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 `@generate_fun_data.sh` around lines 260 - 270, The loop currently runs "$SPECTROMETER" update which can abort the whole script under set -e; change the call inside the while IFS= read -r TAG loop to check the command's exit status and handle failures like the git checkout does: use an if ! "$SPECTROMETER" update --atomvm-dir "$ATOMVM_DIR" --tag "$TAG" --cache "$TMP_CACHE_DIR" --output "$OUTPUT_FILE" --force --no-tests; then echo a warning including the tag and any stderr info and continue to the next tag (or capture $?) — this preserves the existing retry/continue pattern and prevents one bad tag from stopping the whole script.
🤖 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 `@generate_fun_data.sh`:
- Around line 139-154: The loop over tags uses the GNU-only sort -V which breaks
POSIX portability; update the tag enumeration to avoid sort -V by using git's
built-in sorting (e.g. replace git tag -l "v*.*.*" 2>/dev/null | sort -V with
git tag -l --sort=v:refname "v*.*.*" 2>/dev/null) so the for TAG loop (and
writes to TMP_PRERELEASE_TAGS / TMP_RELEASE_TAGS) remains portable, and
optionally add a fallback path that detects lack of --sort and falls back to a
simple numeric/lexical sort or a shell-based version-compare function.
---
Outside diff comments:
In `@generate_fun_data.sh`:
- Around line 260-270: The loop currently runs "$SPECTROMETER" update which can
abort the whole script under set -e; change the call inside the while IFS= read
-r TAG loop to check the command's exit status and handle failures like the git
checkout does: use an if ! "$SPECTROMETER" update --atomvm-dir "$ATOMVM_DIR"
--tag "$TAG" --cache "$TMP_CACHE_DIR" --output "$OUTPUT_FILE" --force
--no-tests; then echo a warning including the tag and any stderr info and
continue to the next tag (or capture $?) — this preserves the existing
retry/continue pattern and prevents one bad tag from stopping the whole script.
🪄 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: ccb6086e-1a65-4dc3-887f-8195f206a610
📒 Files selected for processing (2)
generate_fun_data.shpriv/supported_functions.data
UncleGrumpy
force-pushed
the
WIP
branch
2 times, most recently
from
85ed636 to
5ec61ef
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 @.github/workflows/code_quality_check.yaml:
- Around line 29-33: The "Install shellcheck" CI step can hang on prompts;
update the run block for that step to perform a non-interactive install by
setting DEBIAN_FRONTEND=noninteractive and passing automatic flags to apt-get
(e.g., use apt-get install -y --no-install-recommends and combine update/install
into a single command). Modify the workflow step named "Install shellcheck" (the
run section that currently calls "sudo apt-get update" and "sudo apt-get install
shellcheck") to use those non-interactive flags so the CI step is deterministic.
🪄 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: 0851e993-c00b-49b7-a0f3-8dc6e35b0e67
📒 Files selected for processing (3)
.github/workflows/code_quality_check.yamlgenerate_fun_data.shpriv/supported_functions.data
✅ Files skipped from review due to trivial changes (1)
- priv/supported_functions.data
🚧 Files skipped from review as they are similar to previous changes (1)
- generate_fun_data.sh
Eliminate all bashisms, and include the latest function data updates generated by generate_fun_data.sh. Signed-off-by: Winford <winford@object.stream>
…a.sh Replace the hard-coded list of release tags to scan with a programmatically generated list, only including the most recent pre-release tag for any not yet released versions. Replace the hard-coded use of "release-0.7" with code to determine the latest release-X.X branch at runtime. 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.
Eliminate all bashisms and hard-coded tags and branch names.