chore: consolidate skill catalog based on full stocktake audit#1214
chore: consolidate skill catalog based on full stocktake audit#1214OnoSendai13 wants to merge 2 commits intoaffaan-m:mainfrom
Conversation
- Retire continuous-learning (v1) — fully superseded by v2 with confidence scoring, project isolation, and promotion pipeline - Retire project-guidelines-example — it's a project-specific template (Zenith), not a reusable skill - Retire coding-standards — universal principles merged into rules/common/coding-style.md (KISS, DRY, YAGNI, code smells); language-specific content already covered by frontend-patterns, backend-patterns, and api-design Reduces skill catalog from ~88 to ~85 entries with zero loss of actionable content. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR consolidates coding standards by extending Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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.
🧹 Nitpick comments (1)
rules/common/coding-style.md (1)
27-27: Consider formalizing wording at Line 27.“Gonna” is informal for a standards document. Consider:
YAGNI (You Aren’t Going to Need It).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/common/coding-style.md` at line 27, Update the heading text "YAGNI (You Aren't Gonna Need It)" to use formal wording by changing it to "YAGNI (You Aren’t Going to Need It)"; locate the heading string "YAGNI (You Aren't Gonna Need It)" in rules/common/coding-style.md and replace the informal contraction "Gonna" with "Going to" (keeping the YAGNI acronym intact and preserving punctuation/spacing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rules/common/coding-style.md`:
- Line 27: Update the heading text "YAGNI (You Aren't Gonna Need It)" to use
formal wording by changing it to "YAGNI (You Aren’t Going to Need It)"; locate
the heading string "YAGNI (You Aren't Gonna Need It)" in
rules/common/coding-style.md and replace the informal contraction "Gonna" with
"Going to" (keeping the YAGNI acronym intact and preserving
punctuation/spacing).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 645f087a-15fc-49e7-9675-a3ba89bb32bb
📒 Files selected for processing (6)
rules/common/coding-style.mdskills/coding-standards/SKILL.mdskills/continuous-learning/SKILL.mdskills/continuous-learning/config.jsonskills/continuous-learning/evaluate-session.shskills/project-guidelines-example/SKILL.md
💤 Files with no reviewable changes (5)
- skills/continuous-learning/config.json
- skills/project-guidelines-example/SKILL.md
- skills/coding-standards/SKILL.md
- skills/continuous-learning/evaluate-session.sh
- skills/continuous-learning/SKILL.md
Greptile SummaryThis PR consolidates the skill catalog by retiring three outdated entries —
Confidence Score: 5/5Safe to merge; only documentation/skill files are changed with no runtime impact. All findings are P2 (style-level suggestions on example text). The deletions are well-justified, the content migration is accurate, and no actionable guidance is lost. The single remaining concern — Zenith-specific examples in testing.md — is cosmetic and does not affect correctness. rules/common/testing.md — the 'good' test naming examples should be swapped for domain-neutral ones. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[coding-standards/SKILL.md\nRetired] -->|KISS / DRY / YAGNI| B[rules/common/coding-style.md]
A -->|Naming conventions| B
A -->|Code smells| B
A -->|AAA test structure\nTest naming examples| C[rules/common/testing.md]
A -->|TypeScript / React / Node specifics| D[frontend-patterns\nbackend-patterns\napi-design]
E[continuous-learning/SKILL.md\nRetired v1] -->|Superseded by| F[continuous-learning-v2/SKILL.md\nConfidence scoring\nProject isolation\nPromotion pipeline]
G[project-guidelines-example/SKILL.md\nRetired Zenith template] -.->|⚠️ Zenith examples leaked into| C
Reviews (2): Last reviewed commit: "fix: add TS naming conventions and AAA t..." | Re-trigger Greptile |
Response to PR review feedback on content loss concern: - Add naming conventions to rules/common/coding-style.md: camelCase for vars/functions, PascalCase for types/components, UPPER_SNAKE_CASE for constants, is/has/should/can boolean prefixes, use- prefix hooks - Add AAA (Arrange-Act-Assert) test structure + descriptive test naming examples to rules/common/testing.md This content previously existed in skills/coding-standards/SKILL.md but has no dedicated home in the post-consolidation catalog. Rules are the appropriate layer for language-agnostic naming and test conventions that apply universally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Analysis CompleteGenerated ECC bundle from 10 commits | Confidence: 50% View Pull Request #1215Repository Profile
Detected Workflows (2)
Generated Instincts (8)
After merging, import with: Files
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rules/common/coding-style.md`:
- Line 27: Replace the informal header text "You Aren't Gonna Need It" with the
formal title "You Aren’t Going to Need It (YAGNI)"; locate the header string
exactly as "You Aren't Gonna Need It" in rules/common/coding-style.md and update
it to the new phrasing, ensuring the apostrophe style matches the document's
typography.
- Around line 58-63: The common coding-style section currently mandates
camelCase for variables/functions (`camelCase`, `fetchMarketData`) which
conflicts with Python PEP 8 snake_case; update the document to explicitly state
that language-specific conventions take precedence (e.g., Python uses snake_case
for functions/variables) or move the naming bullets into per-language sections,
and reference the existing examples `camelCase`, `snake_case`, `PascalCase`, and
constants `UPPER_SNAKE_CASE` so reviewers know which rules apply in Python vs
other languages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e7a802be-8948-4f24-9fc0-af9c1c2ad16f
📒 Files selected for processing (2)
rules/common/coding-style.mdrules/common/testing.md
✅ Files skipped from review due to trivial changes (1)
- rules/common/testing.md
| - Avoid copy-paste programming | ||
| - Create reusable abstractions when patterns repeat 3+ times | ||
|
|
||
| ### YAGNI (You Aren't Gonna Need It) |
There was a problem hiding this comment.
Use formal wording in the standards header.
At Line 27, “You Aren't Gonna Need It” is informal for a canonical rule doc; prefer “You Aren’t Going to Need It (YAGNI)”.
🧰 Tools
🪛 LanguageTool
[style] ~27-~27: The word ‘gonna’ is informal.
Context: ... repeat 3+ times ### YAGNI (You Aren't Gonna Need It) - Don't build features before ...
(GONNA)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/common/coding-style.md` at line 27, Replace the informal header text
"You Aren't Gonna Need It" with the formal title "You Aren’t Going to Need It
(YAGNI)"; locate the header string exactly as "You Aren't Gonna Need It" in
rules/common/coding-style.md and update it to the new phrasing, ensuring the
apostrophe style matches the document's typography.
| - **Variables / functions**: `camelCase` with descriptive names (`fetchMarketData`, not `market`) | ||
| - **Booleans**: prefix with `is`, `has`, `should`, `can` (`isUserAuthenticated`, not `flag`) | ||
| - **Interfaces / types**: `PascalCase` (`Market`, `MarketResponse`) | ||
| - **Constants**: `UPPER_SNAKE_CASE` (`MAX_RETRIES`, `DEBOUNCE_DELAY_MS`) | ||
| - **Custom hooks**: `camelCase` with `use` prefix (`useDebounce`, `useAuth`) | ||
| - **Components**: `PascalCase` (`Button`, `MarketCard`) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
`What are the official naming-convention recommendations for:
- Python (PEP 8) for functions/variables
- JavaScript/TypeScript for functions/variables
Please include primary sources.`
💡 Result:
- Python (PEP 8): Functions and variables use lowercase with words separated by underscores (snake_case). 2) JavaScript/TypeScript: Functions and variables use camelCase (starting with lowercase letter, subsequent words capitalized).
Citations:
- 1: https://peps.python.org/pep-0008/
- 2: https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Code_style_guide/JavaScript
- 3: https://google.github.io/styleguide/tsguide.html
🏁 Script executed:
# Check if the file exists and examine its full content
cat -n rules/common/coding-style.md | head -100Repository: affaan-m/everything-claude-code
Length of output: 3306
🏁 Script executed:
# Search for language-specific rule files
find rules -type f -name "*.md" | head -20Repository: affaan-m/everything-claude-code
Length of output: 567
🏁 Script executed:
# Check if Python-specific rules exist
find rules -type d -name "python*"Repository: affaan-m/everything-claude-code
Length of output: 87
🏁 Script executed:
# Check if Python has its own coding-style.md
cat -n rules/python/coding-style.mdRepository: affaan-m/everything-claude-code
Length of output: 1081
Clarify naming convention conflicts with Python PEP 8 in common rules.
At line 58, prescribing camelCase universally conflicts with PEP 8 naming conventions (which require snake_case for functions and variables). The Python rules file references this common file and PEP 8, but doesn't explicitly resolve the contradiction. Add a note clarifying that language-specific rules take precedence, or move naming conventions to language-specific sections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rules/common/coding-style.md` around lines 58 - 63, The common coding-style
section currently mandates camelCase for variables/functions (`camelCase`,
`fetchMarketData`) which conflicts with Python PEP 8 snake_case; update the
document to explicitly state that language-specific conventions take precedence
(e.g., Python uses snake_case for functions/variables) or move the naming
bullets into per-language sections, and reference the existing examples
`camelCase`, `snake_case`, `PascalCase`, and constants `UPPER_SNAKE_CASE` so
reviewers know which rules apply in Python vs other languages.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rules/common/coding-style.md">
<violation number="1" location="rules/common/coding-style.md:62">
P2: React-specific naming guidance was added to a language-agnostic common rules file, conflicting with the documented common-layer scope.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| - **Custom hooks**: `camelCase` with `use` prefix (`useDebounce`, `useAuth`) | ||
| - **Components**: `PascalCase` (`Button`, `MarketCard`) |
There was a problem hiding this comment.
P2: React-specific naming guidance was added to a language-agnostic common rules file, conflicting with the documented common-layer scope.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rules/common/coding-style.md, line 62:
<comment>React-specific naming guidance was added to a language-agnostic common rules file, conflicting with the documented common-layer scope.</comment>
<file context>
@@ -53,6 +53,15 @@ ALWAYS validate at system boundaries:
+- **Booleans**: prefix with `is`, `has`, `should`, `can` (`isUserAuthenticated`, not `flag`)
+- **Interfaces / types**: `PascalCase` (`Market`, `MarketResponse`)
+- **Constants**: `UPPER_SNAKE_CASE` (`MAX_RETRIES`, `DEBOUNCE_DELAY_MS`)
+- **Custom hooks**: `camelCase` with `use` prefix (`useDebounce`, `useAuth`)
+- **Components**: `PascalCase` (`Button`, `MarketCard`)
+
</file context>
| - **Custom hooks**: `camelCase` with `use` prefix (`useDebounce`, `useAuth`) | |
| - **Components**: `PascalCase` (`Button`, `MarketCard`) | |
| - **Framework-specific constructs**: follow naming conventions in the corresponding language/framework rule files (for example, `rules/typescript/*`) | |
|
hey, this branch has merge conflicts with |
|
Closing as partially superseded by direct-port work on main. I ported the reusable common-rule improvements (KISS/DRY/YAGNI, naming conventions, AAA test guidance) but did not accept the broad deletions of coding-standards or continuous-learning v1 as-is. Those removals need a separate deliberate consolidation decision, not a mixed cleanup PR. |
Reduces skill catalog from ~88 to ~85 entries with zero loss of actionable content.
What Changed
Why This Change
Testing Done
node tests/run-all.js)Type of Change
fix:Bug fixfeat:New featurerefactor:Code refactoringdocs:Documentationtest:Testschore:Maintenance/toolingci:CI/CD changesSecurity & Quality Checklist
Documentation
Summary by cubic
Consolidates the skill catalog by retiring three outdated entries and centralizing universal standards in
rules/common/coding-style.mdandrules/common/testing.md. Adds TS naming conventions and the AAA testing pattern to prevent any content loss while reducing the catalog from ~88 to ~85 items.Refactors
continuous-learning(v1); superseded by v2 with confidence scoring, project isolation, and a promotion pipeline.project-guidelines-example; project-specific template, not a reusable skill.coding-standards; merged KISS/DRY/YAGNI, code smells, and TS naming intorules/common/coding-style.md; added AAA test structure and descriptive test naming examples torules/common/testing.md.Migration
continuous-learningv2 and refer torules/common/coding-style.mdandrules/common/testing.mdfor universal standards.Written for commit f5ddc21. Summary will update on new commits.
Summary by CodeRabbit
Documentation
Chores