-
Notifications
You must be signed in to change notification settings - Fork 0
fix(skills): use chained commands in quickstart skill #39
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
base: main
Are you sure you want to change the base?
Conversation
The quickstart skill was showing commands as separate lines, causing iterative execution instead of a single script. This update aligns quickstart with the plan skill by: - Using && to chain all commands into a single executable script - Adding the same domain detection table as plan skill for consistency - Adding explicit instruction to NOT execute commands one-by-one - Trimming ~300 lines of redundant step-by-step instructions - Bumping version from 1.1.0 to 2.0.0 (breaking behavior change) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PR Review: fix(skills): use chained commands in quickstart skillSummaryThis PR significantly improves the quickstart skill by adopting chained commands (using ✅ Strengths
🔍 Code QualityExcellent
🐛 Potential IssuesMinor Concerns:
🚀 Performance ConsiderationsPositive Impact:
No Performance Concerns Identified 🔒 Security ConcernsNo Security Issues The changes don't introduce any security vulnerabilities:
🧪 Test CoverageTest Plan Provided: The PR includes a test plan with 3 items:
Recommendation: Consider adding automated tests for:
📝 Suggestions for Improvement
## IF COMMAND FAILS
If the chained command fails partway through:
1. Check which step failed: `cd [name] && pwd` (did directory get created?)
2. If app was created, cd into it and resume: `lvt gen resource...`
3. If app creation failed, clean up and retry: `rm -rf [name] && [full command]`
| inventory/warehouse | inventory | inventory | items | name:string quantity:int location:string | Password |
| booking/reservation | booking | booking | reservations | name:string date:time status:string | Password |
| event/calendar | events | events | events | title:string date:time location:string | Password |
## DEFAULTS RATIONALE
- Kit: `multi` (full-page layout is most versatile for new apps)
- Seed count: 50 (enough to test pagination, not overwhelming)
- Auth: Password (most commonly needed, easiest to set up)
After successful execution, user should see:
- ✅ "Database migrations applied"
- ✅ "50 [resource] records seeded"
- ✅ "Ready to serve on :8080"🎯 Overall AssessmentRating: ⭐⭐⭐⭐⭐ (5/5) This is an excellent improvement that:
Recommendation: Approve and merge with consideration for the minor suggestions above in a follow-up PR. ✨ Excellent Work!The refactor is well-thought-out and the alignment with the plan skill will make both skills easier to maintain going forward. The reduction in file size without loss of functionality is impressive. Reviewed by: Claude Sonnet 4.5 🤖 |
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.
Pull request overview
This PR refactors the quickstart skill to use chained commands (with &&) instead of separate command lines, aligning it with the plan skill format. The goal is to prevent Claude from executing commands iteratively and instead run them as a single chained script.
Changes:
- Updated quickstart skill to present commands as a single chained command with
&&operators - Added explicit instruction to NOT execute commands one-by-one iteratively
- Removed ~300 lines of redundant documentation and examples while preserving core functionality
| version: 1.1.0 | ||
| version: 2.0.0 | ||
| --- | ||
|
|
Copilot
AI
Jan 14, 2026
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.
The horizontal rule separator uses inconsistent syntax. In markdown, horizontal rules should use three or more hyphens without the 0: prefix. The correct format should be --- on its own line.
| lvt new ___NAME___ --kit multi && \ | ||
| cd ___NAME___ && \ | ||
| lvt gen resource ___RESOURCE___ ___FIELDS___ && \ |
Copilot
AI
Jan 14, 2026
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.
The template shows ___FIELDS___ as a single placeholder, but the FILL IN BLANKS table shows fields like title:string content:text published:bool. The documentation should clarify that multiple space-separated field definitions should replace ___FIELDS___, not just a single value. This could confuse users about the expected format.
| - JUST output the filled template above | ||
| - User can modify after seeing the plan | ||
| - After user says "yes", execute the SINGLE chained command | ||
| - Do NOT execute commands one-by-one iteratively |
Copilot
AI
Jan 14, 2026
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.
Corrected capitalization inconsistency. The instruction uses 'Do NOT' while other similar instructions use 'DO NOT' (all caps).
Summary
&&instead of separate linesProblem
The quickstart skill was showing commands as separate lines:
lvt new [name] --kit multi cd [name] lvt gen resource [resource] [fields...] lvt migration up go mod tidyThis caused Claude to execute commands iteratively, waiting for output after each command instead of running them as a single script.
Solution
Updated to use chained commands matching the plan skill:
Test plan
🤖 Generated with Claude Code