chore: update - #112
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Updates the skills-install Makefile target to continue installing skills after individual failures and report an aggregate result at the end.
Changes:
- Track install failures via a
failedflag instead of exiting on first error. - Update failure messaging to indicate continuation.
- Add a final summary message based on whether any installs failed.
Comments suppressed due to low confidence (1)
Makefile:1
- The
failedflag is set inside awhileloop that is fed by a pipeline (... | while ...), which runs the loop in a subshell in many/bin/shimplementations. As a result,failed=1won’t reliably persist to line 114, so the target may report success even when installs failed. To make this work reliably, avoid piping into thewhileloop (e.g., read the file directly and skip comments/blank lines inside the loop, or otherwise structure it so the loop runs in the main shell), and consider returning a non-zero exit status when any install failed somake/CI can detect the failure.
.DEFAULT_GOAL := help
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| skills-install: ## Install skills from SKILLS.txt (supports per-repo skill selection). | ||
| @grep -v '^\s*#' $(SKILLS_FILE) | grep -v '^\s*$$' | while IFS= read -r line; do \ | ||
| @failed=0; \ | ||
| grep -v '^\s*#' $(SKILLS_FILE) | grep -v '^\s*$$' | while IFS= read -r line; do \ |
There was a problem hiding this comment.
The grep patterns use \s (e.g., ^\s*#), which is not portable in POSIX/basic grep and is typically treated as a literal s unless using PCRE modes. This can cause comment/blank-line filtering to behave incorrectly on some systems. Prefer a POSIX-compatible character class like [[:space:]] (e.g., ^[[:space:]]*# and ^[[:space:]]*$$) or move this filtering into the shell loop with a case check.
| grep -v '^\s*#' $(SKILLS_FILE) | grep -v '^\s*$$' | while IFS= read -r line; do \ | |
| grep -v '^[[:space:]]*#' $(SKILLS_FILE) | grep -v '^[[:space:]]*$$' | while IFS= read -r line; do \ |
Summary by cubic
Update the
skills-installMakefile target to continue installing other repos when one fails and show a final summary. This avoids aborting on the first error and makes bulk installs more reliable.failedflag to keep processing remaining repos.Written for commit 545ae21. Summary will update on new commits.