-
Notifications
You must be signed in to change notification settings - Fork 19
build(make): modularize prerequisites checks into subtasks #754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request refactors the prerequisites validation system by replacing a monolithic check with modular individual prerequisite verification tasks for npx, awk, perl, and jq, combined into a composite check-prerequisites task that is now a dependency for docs generation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant docs-generate
participant check-prerequisites
participant check-npx
participant check-awk
participant check-perl
participant check-jq
User->>docs-generate: Run task
docs-generate->>check-prerequisites: Execute dependency
par Parallel checks
check-prerequisites->>check-npx: Verify npx
check-prerequisites->>check-awk: Verify awk
check-prerequisites->>check-perl: Verify perl
check-prerequisites->>check-jq: Verify jq
and
check-npx-->>check-prerequisites: Success/Fail
check-awk-->>check-prerequisites: Success/Fail
check-perl-->>check-prerequisites: Success/Fail
check-jq-->>check-prerequisites: Success/Fail
end
check-prerequisites-->>docs-generate: All checks passed
docs-generate->>docs-generate: Execute docs generation
docs-generate-->>User: Complete
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches✅ Passed checks (3 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
Makefile.toml (4)
98-106
: Use POSIX-safe command detection and refine install hintPrefer
command -v
with stderr redirected to avoid noisy “which: no npx” output; tweak install guidance.-if ! which npx >/dev/null; then - echo "❌ npx could not be found" - echo " Consider installing npx with: npm install -g npx" +if ! command -v npx >/dev/null 2>&1; then + echo "❌ npx could not be found" + echo " Install Node.js (includes npm and typically npx). If needed: npm install -g npx" exit 1 fi
108-117
: Silence detection errors; prefercommand -v
overwhich
Avoid stderr noise and improve portability.
-if ! which awk >/dev/null; then +if ! command -v awk >/dev/null 2>&1; then echo "❌ awk could not be found" echo " Consider installing awk (usually pre-installed on Unix systems)" exit 1 fi
118-127
: Mirror detection fix for perlUse
command -v
and silence stderr as above.-if ! which perl >/dev/null; then +if ! command -v perl >/dev/null 2>&1; then echo "❌ perl could not be found" echo " Consider installing perl (usually pre-installed on Unix systems)" exit 1 fi
151-156
: Composite prerequisites task looks good; consider reusing checks where usedLGTM. Since several chain tasks pipe output through jq, consider adding
dependencies = ["check-jq"]
to those tasks to fail fast when jq is missing.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Makefile.toml
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.toml
⚙️ CodeRabbit configuration file
Review the TOML configuration files for correctness, maintainability, and adherence to best practices.
Files:
Makefile.toml
⏰ 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). (3)
- GitHub Check: report-wasm-size
- GitHub Check: test-rust
- GitHub Check: Analyze (rust)
size-limit report 📦
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
Split monolithic
check-prerequisites
task into individual subtasks. Each tool (npx
,awk
,perl
,jq
) now has itsown check task. Also fix
jq
version validation to support1.7+
(instead of strict1.7
).Summary by CodeRabbit