Skip to content

Conversation

@jwm4
Copy link
Contributor

@jwm4 jwm4 commented Dec 4, 2025

See #367 (comment) for context.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

Claude Code Review

Summary

PR #420 adds a new Documentation Standards section to CLAUDE.md that provides guidance on where and how to add documentation. This change addresses the issue raised in #367 about preventing "slop markdown files" (unnecessary top-level documentation proliferation).

Overall Assessment:APPROVED - This is a valuable addition that aligns perfectly with the project's documentation philosophy and established patterns.

Issues by Severity

🚫 Blocker Issues

None.

🔴 Critical Issues

None.

🟡 Major Issues

None.

🔵 Minor Issues

1. Placement within CLAUDE.md

The new section is placed under "Documentation Structure" (line 988), which is appropriate. However, it could be even more discoverable by:

  • Adding a reference to it in the Table of Contents (if one exists)
  • Cross-referencing it in the "Frontend Development Standards" section which already has similar "Pre-Commit Checklist" guidance

Recommendation: Consider adding a brief note in the Memory System section pointing to these documentation standards when working on docs.

Positive Highlights

Excellent consistency - The new section follows the exact same style and formatting as other sections in CLAUDE.md (e.g., Frontend Development Standards)

Actionable guidance - Clear, concrete rules with good examples:

  • "Prefer inline updates" vs. "Avoid top-level proliferation"
  • Specific file references (docs/amber-quickstart.md, components/backend/README.md)

Addresses real problem - This directly tackles the issue from #367 about documentation sprawl

Well-placed - Logically positioned within the "Documentation Structure" section, making it easy to find

Aligns with existing patterns - The guidance matches how documentation is already organized in the repo (colocated component READMEs, memory system in .claude/, etc.)

Recommendations

Immediate Action Items

None required - this PR is ready to merge as-is.

Future Enhancements (Optional)

  1. Add to Pre-Commit Checklist: Consider adding a documentation-specific checklist item in the relevant sections

  2. Memory System Integration: If creating a documentation-focused memory file in the future, reference these standards there.

  3. Automated Checks: Consider a lightweight CI check that warns if new .md files are added to the root (could be a simple GitHub Action).

Conclusion

This PR makes a simple but important improvement to CLAUDE.md by codifying best practices for documentation organization. The change is well-written, appropriately placed, and directly addresses documented concerns from previous discussions.

Recommendation:Merge


Review performed by Claude Code using project standards from CLAUDE.md and memory system context files.


🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

  1. CLAUDE.md - Master project instructions, development standards
  2. backend-development.md - Go backend, K8s integration patterns
  3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
  4. security-standards.md - Auth, RBAC, token handling
  5. k8s-client-usage.md - User token vs service account patterns
  6. error-handling.md - Consistent error patterns
  7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

@jeremyeder
Copy link
Collaborator

i like it

@jeremyeder jeremyeder merged commit e589f3c into ambient-code:main Dec 4, 2025
20 checks passed
@jwm4 jwm4 deleted the patch-1 branch December 4, 2025 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants