🥯 [just] v4.2: script for installing pre-requisites #48
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new prerequisite installation script for the just command runner, versioned as v4.2. The script automates the detection and installation of development tools required for just recipes.
- Adds a comprehensive bash script that checks for and installs missing prerequisites (just, gh, shellcheck, markdownlint-cli2, jq)
- Provides platform-specific installation logic for macOS and Linux with multiple package manager support
- Updates the PR workflow version reference to v4.2
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .just/install-prerequisites.sh | New script that detects OS, checks for missing prerequisites, and provides installation instructions or automated installation for supported platforms |
| .just/gh-process.just | Updates version comment from v4.1 to v4.2 to reflect the new prerequisite installation feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo " wget -qO - 'https://proget.makedeb.org/debian-feeds/prebuilt-mpr.pub' | gpg --dearmor | sudo tee /usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg 1> /dev/null" | ||
| echo " echo \"deb [arch=all,\$(dpkg --print-architecture) signed-by=/usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg] https://proget.makedeb.org prebuilt-mpr \$(lsb_release -cs)\" | sudo tee /etc/apt/sources.list.d/prebuilt-mpr.list" | ||
| echo " sudo apt update && sudo apt install just" |
There was a problem hiding this comment.
These complex multi-line installation commands are difficult to read and maintain. Consider breaking them into separate echo statements or storing them in a heredoc for better readability.
| echo " wget -qO - 'https://proget.makedeb.org/debian-feeds/prebuilt-mpr.pub' | gpg --dearmor | sudo tee /usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg 1> /dev/null" | |
| echo " echo \"deb [arch=all,\$(dpkg --print-architecture) signed-by=/usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg] https://proget.makedeb.org prebuilt-mpr \$(lsb_release -cs)\" | sudo tee /etc/apt/sources.list.d/prebuilt-mpr.list" | |
| echo " sudo apt update && sudo apt install just" | |
| cat <<'EOF' | |
| wget -qO - 'https://proget.makedeb.org/debian-feeds/prebuilt-mpr.pub' | gpg --dearmor | sudo tee /usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg 1> /dev/null | |
| echo "deb [arch=all,$(dpkg --print-architecture) signed-by=/usr/share/keyrings/prebuilt-mpr-archive-keyring.gpg] https://proget.makedeb.org prebuilt-mpr $(lsb_release -cs)" | sudo tee /etc/apt/sources.list.d/prebuilt-mpr.list | |
| sudo apt update && sudo apt install just | |
| EOF |
| elif [[ "$PKG_MGR" == "dnf" ]]; then | ||
| echo -e "${CYAN}Install shellcheck:${NC} sudo dnf install shellcheck" | ||
| elif [[ "$PKG_MGR" == "pacman" ]]; then | ||
| echo -e "${CYAN}Install shellcheck:${NC} sudo pacman -S shellcheck" |
There was a problem hiding this comment.
The shellcheck installation case is missing a fallback message for manual installation when PKG_MGR is 'manual', unlike the other tools. Consider adding consistency by including an else clause with a reference to shellcheck's installation documentation.
| echo -e "${CYAN}Install shellcheck:${NC} sudo pacman -S shellcheck" | |
| echo -e "${CYAN}Install shellcheck:${NC} sudo pacman -S shellcheck" | |
| else | |
| echo -e "${CYAN}Install shellcheck:${NC} See https://github.com/koalaman/shellcheck#installing" |
| elif [[ "$PKG_MGR" == "dnf" ]]; then | ||
| echo -e "${CYAN}Install jq:${NC} sudo dnf install jq" | ||
| elif [[ "$PKG_MGR" == "pacman" ]]; then | ||
| echo -e "${CYAN}Install jq:${NC} sudo pacman -S jq" |
There was a problem hiding this comment.
The jq installation case is missing a fallback message for manual installation when PKG_MGR is 'manual', unlike the just tool which provides a link. Consider adding an else clause for consistency.
| echo -e "${CYAN}Install jq:${NC} sudo pacman -S jq" | |
| echo -e "${CYAN}Install jq:${NC} sudo pacman -S jq" | |
| else | |
| echo -e "${CYAN}Install jq manually:${NC} See https://jqlang.github.io/jq/download/" |
Code Review - PR #48: Install Prerequisites ScriptOverviewThis PR adds a useful prerequisite installation script ( Strengths ✅
Issues Found 🔴1. Script not integrated into justfile (Severity: Medium)The script exists but isn't callable via Recommendation: Add a recipe to the main # install development prerequisites
[group('Utility')]
install_prereqs:
.just/install-prerequisites.sh2. Missing from documentation (Severity: Medium)CLAUDE.md doesn't mention this new script. New users won't know it exists. Recommendation: Add to CLAUDE.md under "Additional commands" section or create a setup/getting started section. 3. Release notes not updated (Severity: Low)The version bump to v4.2 in Recommendation: Add a v4.2 entry to RELEASE_NOTES.md documenting this feature. 4. Linux behavior inconsistency (Severity: Low)macOS auto-installs tools, but Linux only shows commands. While this is a reasonable choice (sudo access varies), it could confuse users who expect consistent behavior. Consider: Adding a 5. No validation of installed tools (Severity: Low)After macOS installation, the script doesn't verify the tools are actually working, only that the installation commands succeeded. Consider: Re-running the detection checks after installation to confirm success. 6. GitHub CLI authentication reminder (.just/install-prerequisites.sh:94, :153)The script reminds users to run Consider: Add Security Considerations 🔒
Performance Considerations ⚡No concerns - script is lightweight and runs standard system commands. Test Coverage 📊Issue: No tests exist for this script. The repository has no test framework at all. Recommendation: Consider adding basic tests:
For a template repo, comprehensive testing may be overkill, but at minimum:
Code Quality 🎨The bash code is clean and well-structured:
Minor suggestion: The script could benefit from a Recommendations SummaryMust fix before merge:
Should fix:
Nice to have:
ConclusionThis is a solid addition that will help new users get started faster. The code quality is good and follows bash best practices. The main gaps are in integration and documentation rather than the script itself. With the integration and documentation updates, this will be a valuable addition to the template. Review automated via Claude Code |
Code Review - PR #48SummaryThis PR adds a well-crafted prerequisites installation script ( ✅ Strengths1. Code Quality
2. Cross-Platform Support
3. User Experience
4. Documentation
🔍 Areas for Consideration1. Script Behavior Inconsistency (Linux vs macOS) The script has different behaviors on Linux vs macOS:
This inconsistency could confuse users. Consider either:
Reference: Lines 76-114 (macOS auto-installs) vs 117-182 (Linux shows commands) 2. Missing Error Handling for Failed Installations On macOS, if a
Reference: .just/install-prerequisites.sh:85-114 3. npm Global Install Considerations Line 107 uses
Consider adding:
Reference: .just/install-prerequisites.sh:107 4. Hardcoded Package Manager Commands The apt-get installation for
Consider mentioning alternative installation methods or linking to official docs. Reference: .just/install-prerequisites.sh:138-142 🔒 Security✅ No significant security concerns
Minor note: The wget/gpg command chain on line 140 pipes directly to system locations. This is standard practice but users should verify the source is trusted. ⚡ Performance✅ Efficient implementation
🧪 Test CoverageCurrent state: No automated tests for the shell script Recommendations:
However, given this is a developer tooling script that will be run manually, extensive testing may be overkill. The shellcheck validation provides good static analysis coverage. 📝 Documentation & Release Notes✅ Excellent documentation
Minor suggestion: Consider adding a usage example in the README or a comment at the top of the script itself showing: # Usage: ./.just/install-prerequisites.sh
# This script checks for required tools and helps install them🎯 Overall AssessmentStatus: ✅ Approved with minor suggestions This is a high-quality contribution that adds real value for repository onboarding. The code is clean, well-structured, and thoughtfully implemented. The inconsistency between macOS and Linux behavior is the main item worth addressing, but it's not a blocker. Suggested improvements (non-blocking):
Nice touches:
Great work! 🎉 |
Code Review - PR #48OverviewThis PR adds a comprehensive prerequisites installation script ( ✅ Strengths
🔍 Minor Observations
🛡️ Security
🧪 Test Coverage
📝 Documentation
RecommendationApprove with optional minor enhancement. The code is production-ready as-is. The only suggestion is to add Overall, this is a well-crafted contribution that significantly improves the onboarding experience for new contributors. Great work! 🎯 Review generated by Claude Code |
Done
Meta
(Automated in
.just/gh-process.just.)