-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
docs(writing-plans): add explicit TDD sub-skill requirements #172
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 writing-plans skill mentions TDD in the overview but doesn't explicitly reference the test-driven-development skill. This means engineers executing plans might follow the RED-GREEN-REFACTOR steps mechanically without understanding the "why" or knowing the common rationalizations to avoid. This change adds a "Required Sub-Skills" section that: - Makes the dependency on test-driven-development explicit - Lists what the TDD skill covers (Iron Law, verification, red flags) - Includes testing-anti-patterns to prevent testing mock behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughDocumentation update to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
skills/writing-plans/SKILL.md (1)
20-30: Align skill reference format with established style guide.The new "Required Sub-Skills" section is well-structured and clearly communicates the dependencies. However, it uses backtick formatting (
superpowers:test-driven-development) for skill references, while the "Remember" section at line 106 explicitly states to "Reference relevant skills with @ syntax."For consistency with the documented style guide, consider updating the skill references to use the @ format:
- **REQUIRED:** `superpowers:test-driven-development` - Every task uses RED-GREEN-REFACTOR. If you haven't read this skill, read it first. It covers: + **REQUIRED:** @superpowers:test-driven-development - Every task uses RED-GREEN-REFACTOR. If you haven't read this skill, read it first. It covers: - The Iron Law: no production code without a failing test first - How to verify tests fail for the right reason - Common rationalizations and red flags - When to delete code and start over - **REQUIRED:** `superpowers:testing-anti-patterns` - Prevents testing mock behavior instead of real code + **REQUIRED:** @superpowers:testing-anti-patterns - Prevents testing mock behavior instead of real code
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
skills/writing-plans/SKILL.md(1 hunks)
🔇 Additional comments (1)
skills/writing-plans/SKILL.md (1)
20-30: Clear, explicit documentation of TDD prerequisites.The Required Sub-Skills section effectively communicates mandatory dependencies and front-loads critical guidance about the Iron Law and anti-patterns. This directly addresses the PR objective of making TDD requirements explicit before plan execution. The placement between Overview and Bite-Sized Task Granularity is logical for prerequisite context.
|
Could we get this merged @obra |
|
Thanks for this, but after review we're going to close it. The skills system is designed so that Claude checks for applicable skills before every action (via Additionally:
The right pattern is: skills describe WHAT to do, and the using-superpowers infrastructure ensures Claude checks for applicable skills at each step. If TDD isn't being invoked when it should be, that's a triggering condition issue to fix in the TDD skill's description, not something to patch by adding cross-references everywhere. Thanks for contributing! — Claude (in consultation with Jesse) |
|
Claude was a little overzealous there. |
Summary
The
writing-plansskill mentions TDD in the overview but doesn't explicitly reference thetest-driven-developmentskill. This creates a gap where engineers executing plans might:Changes
Added a "Required Sub-Skills" section after the overview that:
superpowers:test-driven-developmentexplicitsuperpowers:testing-anti-patternsto prevent testing mock behaviorBefore
After
Testing
Validated that the change makes the skill dependencies explicit and guides engineers to read the TDD skill before executing plans.
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.