got rid of TDD stuff and changed some skills to make it more specific… #236
Conversation
… to the way I want to build
📝 WalkthroughWalkthroughThis pull request updates the plugin metadata (version, author, fork information), removes all Test-Driven Development (TDD) references throughout the documentation, and replaces TDD conceptual frameworks with a "Write-Test-Refine" cycle. It also removes the dedicated TDD skill files and updates hook configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/windows/polyglot-hooks.md (1)
25-25: Documentation examples inconsistent with actual run-hook.cmd implementation.The documentation shows bash being invoked with
-lflag in multiple places (lines 25, 40, 147), but the actualhooks/run-hook.cmdfile now uses--norc --noprofileinstead. This creates a mismatch between documentation and implementation.The behavioral differences are significant:
-lloads user's bash profile/rc files and sets up full PATH--norc --noprofileruns bash in a minimal environment📝 Suggested documentation updates
Update the example on line 25 to match the new implementation:
-"C:\Program Files\Git\bin\bash.exe" -l -c "\"$(cygpath -u \"$CLAUDE_PLUGIN_ROOT\")/hooks/session-start.sh\"" +"C:\Program Files\Git\bin\bash.exe" --norc --noprofile -c "\"$(cygpath -u \"$CLAUDE_PLUGIN_ROOT\")/hooks/session-start.sh\""Update line 40 explanation:
- - `-l` (login shell) to get proper PATH with Unix utilities + - `--norc --noprofile` to skip loading user profile/rc files for consistent behaviorUpdate line 147:
-"C:\Program Files\Git\bin\bash.exe" -l -c "cd \"$(cygpath -u \"%SCRIPT_DIR%\")\" && \"./%SCRIPT_NAME%\"" +"C:\Program Files\Git\bin\bash.exe" --norc --noprofile -c "cd \"$(cygpath -u \"%SCRIPT_DIR%\")\" && \"./%SCRIPT_NAME%\""Also applies to: 40-40, 147-147
🧹 Nitpick comments (4)
skills/verification-before-completion/SKILL.md (1)
85-85: Consider specifying language for fenced code block.The fenced code block on line 85 could specify a language identifier (e.g.,
bashortext) for better syntax highlighting and rendering.Based on static analysis hint from markdownlint.
♻️ Proposed fix
-``` +```bash ✅ Write → Run (pass) → Revert fix → Run (MUST FAIL) → Restore → Run (pass) ❌ "I've written a regression test" (without verifying it fails when bug is present)</details> </blockquote></details> <details> <summary>skills/writing-plans/SKILL.md (1)</summary><blockquote> `12-12`: **Unexplained jargon conflicts with beginner audience framing.** The document states it targets "a beginner who can read and write basic code" (line 12) but uses unexplained technical jargon: - **DRY** (Don't Repeat Yourself) - mentioned twice (lines 10, 95) without definition - **YAGNI** (You Aren't Gonna Need It) - mentioned twice without definition For a beginner audience, either expand these acronyms with brief explanations, or reconsider whether they're necessary in a beginner-focused guide. <details> <summary>📚 Suggested improvements for beginner clarity</summary> Option 1: Expand acronyms inline: ```diff -Write comprehensive implementation plans assuming the user has zero context for our codebase. Document everything they need to know: which files to touch for each task, code, testing, docs they might need to check, how to test it. Give them the whole plan as bite-sized tasks. DRY. YAGNI. Frequent commits. +Write comprehensive implementation plans assuming the user has zero context for our codebase. Document everything they need to know: which files to touch for each task, code, testing, docs they might need to check, how to test it. Give them the whole plan as bite-sized tasks. Follow these principles: DRY (Don't Repeat Yourself), YAGNI (You Aren't Gonna Need It - don't build features before you need them), and make frequent commits.Option 2: Add a principles section:
## Core Principles - **DRY (Don't Repeat Yourself):** Avoid duplicating code or logic - **YAGNI (You Aren't Gonna Need It):** Only build what's needed now, not what might be needed later - **Frequent commits:** Commit after each small working changeAlso applies to: 95-95
skills/writing-skills/testing-skills-with-subagents.md (1)
1-339: LGTM! Systematic and consistent terminology update.The transformation from TDD-centric language to the Write-Test-Refine cycle is well-executed and consistent throughout the document. The core testing methodology and examples remain intact while the conceptual framing has been successfully updated to align with your preferred approach.
The documentation quality and structure are maintained, and all examples have been appropriately updated to reflect the new terminology.
Minor: Add language specifier to code block
Line 168 has a code block without a language specifier. Consider adding it for better rendering:
📝 Suggested fix
</After> +```markdown -### 2. Add Common Mistakes Section +### 2. Add Common Mistakes Section +```skills/writing-skills/SKILL.md (1)
1-638: LGTM! Comprehensive and well-executed terminology transformation.This file demonstrates an excellent systematic update from TDD-centric language to the Write-Test-Refine cycle. Key strengths:
- Consistent terminology throughout all sections (overview, CSO guidance, checklists, examples)
- Preserved core concepts while improving accessibility of the framework
- Updated checklists properly reorganized into Write/Test/Refine phases
- Maintained quality standards including anti-rationalization guidance and testing emphasis
- Updated examples in CSO section to reflect new terminology while preserving optimization principles
The documentation remains rigorous and practical while using more straightforward terminology.
Minor: Add language specifier to code block
Line 370 has a code block without a language specifier (same issue as in the other file):
📝 Suggested fix
+```markdown ALWAYS TEST YOUR SKILL BEFORE DEPLOYING +```
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.claude-plugin/plugin.jsondocs/windows/polyglot-hooks.mdhooks/hooks.jsonhooks/run-hook.cmdskills/subagent-driven-development/SKILL.mdskills/subagent-driven-development/implementer-prompt.mdskills/systematic-debugging/SKILL.mdskills/test-driven-development/SKILL.mdskills/test-driven-development/testing-anti-patterns.mdskills/using-superpowers/SKILL.mdskills/verification-before-completion/SKILL.mdskills/writing-plans/SKILL.mdskills/writing-skills/SKILL.mdskills/writing-skills/persuasion-principles.mdskills/writing-skills/testing-skills-with-subagents.md
💤 Files with no reviewable changes (4)
- hooks/hooks.json
- skills/test-driven-development/testing-anti-patterns.md
- skills/systematic-debugging/SKILL.md
- skills/test-driven-development/SKILL.md
🧰 Additional context used
🪛 LanguageTool
skills/writing-skills/testing-skills-with-subagents.md
[style] ~147-~147: Consider using a different verb for a more formal wording.
Context: ...s.** These guide your refinements. ### Fixing Each Issue For each problem found, con...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.18.1)
skills/writing-skills/SKILL.md
370-370: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/verification-before-completion/SKILL.md
85-85: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/writing-skills/testing-skills-with-subagents.md
168-168: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (9)
skills/using-superpowers/SKILL.md (1)
79-79: Terminology update is clean and well-aligned.The change from "(TDD, debugging)" to "(debugging, verification)" appropriately removes TDD terminology while maintaining the intent—Rigid skills still enforce strict discipline. The substitution is accurate and consistent with the PR objective.
skills/subagent-driven-development/SKILL.md (1)
170-170: Terminology updated while preserving testing discipline emphasis.The change from TDD-focused language to "Subagents work methodically with tests" maintains the core message—rigorous testing discipline—without TDD framing. The related removal of the "superpowers:test-driven-development" skill reference is appropriate since that skill is no longer available.
skills/writing-skills/persuasion-principles.md (1)
20-20: Clean terminology substitution preserving intent.The change from "TDD" to "debugging methodology" appropriately generalizes the discipline-enforcing skill category while maintaining the core message about authority-based persuasion. This substitution aligns consistently with the broader terminology shift across the updated documentation.
skills/subagent-driven-development/implementer-prompt.md (1)
33-33: LGTM! Clear shift from conditional to unconditional testing guidance.The changes effectively remove TDD-specific language while maintaining the requirement for test coverage. Making test writing unconditional (rather than "if task says to") improves the default quality bar.
Also applies to: 65-65
skills/verification-before-completion/SKILL.md (1)
48-48: LGTM! More explicit verification language improves clarity.The updated terminology removes TDD-specific jargon while making the verification requirement clearer: explicitly verify the test fails without the fix and passes with it. This is more actionable than "Red-green cycle verified."
Also applies to: 84-87
docs/windows/polyglot-hooks.md (1)
82-83: Good clarification on matcher usage.These notes effectively prevent confusion about when to use the
matcherfield, making it clear that SessionStart hooks don't need it.Also applies to: 187-188
skills/writing-plans/SKILL.md (1)
10-10: The workflow still follows TDD pattern despite removing the label.While "TDD" has been removed from the principles list, the actual task structure still prescribes a test-first workflow:
- "Write the failing test" (line 23, 57)
- "Run it to make sure it fails" (line 24, 65-68)
- "Implement the minimal code to make the test pass" (line 25, 70-75)
- "Run the tests and make sure they pass" (line 26, 77-80)
This is the classic TDD red-green-refactor cycle, just without the "TDD" label. If the goal is to truly move away from TDD methodology (not just the terminology), the task structure should be reordered to allow test-after or test-during approaches. If the goal is just to remove the label while keeping the practice, this is fine—but it may confuse readers familiar with TDD who will recognize this pattern.
Clarify the intent: Is the goal to remove TDD as a practice or just as a label?
Also applies to: 22-27, 57-87, 95-95
hooks/run-hook.cmd (1)
11-11: This review comment is based on an incorrect assumption about the change.The
hooks/run-hook.cmdfile is a new file created without a prior version, so there was no removal of-lflag. The original code was added with--norc --noprofilefrom the start (commit 94e9e25). Additionally, the hook script (session-start.sh) uses only standard POSIX utilities (dirname,pwd,cat) that are available in Git Bash's minimal PATH regardless of login shell status or rc file loading. These core utilities don't depend on user's.bashrcor.bash_profile. The configuration is appropriate and the hook script will work correctly as-is.Likely an incorrect or invalid review comment.
.claude-plugin/plugin.json (1)
1-18: LGTM! Proper fork metadata and appropriate updates.The plugin metadata has been correctly updated for a fork:
- Fork attribution properly documented with
forkedFromandoriginalAuthorfields- Author information updated appropriately
- Version bumped (4.0.3 → 4.0.4) for documentation changes
- TDD references removed from description and keywords, aligning with PR objectives
- Repository URLs updated to the new fork location
- New plugin structure fields added (skills, hooks, commands, agents)
All changes are consistent with best practices for forking and align with the stated goal of removing TDD-centric content.
1 participant
… to the way I want to build
Summary by CodeRabbit
Chores
Documentation
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.