docs: split CLAUDE.md into focused skills#122
Open
feloy wants to merge 4 commits intokortex-hub:mainfrom
Open
Conversation
CLAUDE.md was too large (1814 lines) making it difficult to navigate and find specific guidance. Split detailed implementation guides into 9 new skills while keeping essential architectural patterns in core docs. Reduces CLAUDE.md to 348 lines (81% reduction). New skills created: - working-with-runtime-system - Runtime architecture and optional interfaces - working-with-steplogger - Progress feedback integration - working-with-config-system - Workspace configuration and multi-level merging - working-with-podman-runtime-config - Podman runtime configuration - implementing-command-patterns - Advanced command patterns - testing-commands - Command testing patterns - working-with-instances-manager - Manager API and project detection - cross-platform-development - Path handling and testing practices - testing-best-practices - Parallel tests and fake objects Closes kortex-hub#112 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 4 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughExtracted large sections from AGENTS.md into eight new skill documents and added nine small pointer files under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 5
🧹 Nitpick comments (2)
skills/implementing-command-patterns/SKILL.md (1)
42-43: Resolve guidance inconsistency for flag access.
The doc says to always bind flags directly to struct fields, but the JSON pattern then reads--storageviacmd.Flags().GetString(). Please clarify this as an explicit exception for inherited/global flags, or update the wording from “always” to avoid contradiction.Based on learnings: "Applies to pkg/cmd/*.go : Always bind command flags directly to struct fields using
*Varvariants (StringVarP, BoolVarP, IntVarP) instead of using non-binding variants".Also applies to: 163-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/implementing-command-patterns/SKILL.md` around lines 42 - 43, The doc currently mandates "always" binding flags with the *Var variants (e.g., StringVarP, BoolVarP, IntVarP) but the JSON example still reads an inherited/global flag using cmd.Flags().GetString() (the --storage access), creating a contradiction; update the prose to remove the absolute "always" and add a single explicit exception that inherited/global or shared flags may be read via cmd.Flags().GetString()/GetBool()/GetInt when they cannot be bound to the local command struct, and update the JSON example or its caption to either bind --storage with StringVarP to the command struct (preferred) or call out that cmd.Flags().GetString() is an intentional exception for inherited/global flags (mentioning cmd.Flags().GetString() and StringVarP as the symbols to inspect).skills/testing-best-practices/SKILL.md (1)
35-53: Clarify example names in “correct vs incorrect”t.Setenv()snippets.
Using the same function name for both examples can be confusing when copied. Consider distinct names likeTestWithEnvVariable_CorrectandTestWithEnvVariable_Incorrect.Based on learnings: "Applies to **/*_test.go : Tests using
t.Setenv()to set environment variables CANNOT uset.Parallel()on the parent test function due to Go testing framework restrictions".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/testing-best-practices/SKILL.md` around lines 35 - 53, Rename the duplicate test function names so the correct and incorrect examples are unambiguous (e.g., change the first TestWithEnvVariable to TestWithEnvVariable_Correct and the second to TestWithEnvVariable_Incorrect), and update the surrounding text to call out the rule applies to tests in *_test.go (tests that use t.Setenv() on a parent must not call t.Parallel()); update references in the examples to match the new identifiers (TestWithEnvVariable_Correct, TestWithEnvVariable_Incorrect) so copying them won’t produce duplicate-symbol errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/implementing-command-patterns/SKILL.md`:
- Around line 255-257: Remove the ignored-error pattern when reading the
--storage flag and resolving its path: call cmd.Flags().GetString("storage") and
check its error, and either remove the unnecessary filepath.Abs() or call
filepath.Abs(storageDir) and check its error; update usages of
storageDir/absStorageDir (e.g., when constructing instances.NewManager) to use
the validated variable. Ensure you return wrapped errors like "failed to read
--storage flag" or "failed to resolve storage path" so failures are propagated
instead of being ignored.
In `@skills/working-with-config-system/SKILL.md`:
- Around line 51-55: The heading sentence "Configurations are merged from lowest
to highest priority" contradicts the numbered list order; update SKILL.md to
make them consistent by either changing that sentence to "highest to lowest
priority" or reversing the numbered list so it reads Workspace → Global project
→ Project-specific → Agent-specific; reference the exact phrases to adjust: the
sentence "Configurations are merged from lowest to highest priority" and the
list items "Agent-specific configuration (from `agents.json`)",
"Project-specific configuration (from `projects.json` using project ID)",
"Global project configuration (from `projects.json` using empty string `\"\"`
key)", and "Workspace-level configuration (from `.kortex/workspace.json`)".
In `@skills/working-with-instances-manager/SKILL.md`:
- Around line 257-260: The code is referring to the wrong package for the
ErrRuntimeNotFound error; replace the incorrect qualifier
instances.ErrRuntimeNotFound with runtime.ErrRuntimeNotFound (and ensure the
pkg/runtime package is imported or referenced appropriately) in the runtime not
found check using the symbol ErrRuntimeNotFound so the code compiles and matches
the actual error definition.
In `@skills/working-with-runtime-system/SKILL.md`:
- Around line 108-123: The Terminal method signature on podmanRuntime must be
updated to match the interface by adding the missing agent parameter: change the
function signature of Terminal to accept (ctx context.Context, agent string,
instanceID string, command []string) error, add a simple validation for agent
(e.g., ensure agent != "" similar to instanceID/command checks), and keep the
existing arg construction and invocation of p.executor.RunInteractive(ctx,
args...) (no other call sites in this snippet need code changes here). Ensure
you update the podmanRuntime.Terminal function declaration and its parameter
usage so the method matches the interface while continuing to build args :=
[]string{"exec","-it", instanceID} and calling p.executor.RunInteractive.
- Around line 98-104: Update the documented Terminal interface signature in
SKILL.md to match the actual code by adding the missing agent parameter: the
Terminal method on the Terminal interface should accept (ctx context.Context,
agent string, instanceID string, command []string) so the doc reflects the
4-parameter signature used to load agent-specific configuration for the terminal
session; update the comment text if needed to mention the agent argument's
purpose.
---
Nitpick comments:
In `@skills/implementing-command-patterns/SKILL.md`:
- Around line 42-43: The doc currently mandates "always" binding flags with the
*Var variants (e.g., StringVarP, BoolVarP, IntVarP) but the JSON example still
reads an inherited/global flag using cmd.Flags().GetString() (the --storage
access), creating a contradiction; update the prose to remove the absolute
"always" and add a single explicit exception that inherited/global or shared
flags may be read via cmd.Flags().GetString()/GetBool()/GetInt when they cannot
be bound to the local command struct, and update the JSON example or its caption
to either bind --storage with StringVarP to the command struct (preferred) or
call out that cmd.Flags().GetString() is an intentional exception for
inherited/global flags (mentioning cmd.Flags().GetString() and StringVarP as the
symbols to inspect).
In `@skills/testing-best-practices/SKILL.md`:
- Around line 35-53: Rename the duplicate test function names so the correct and
incorrect examples are unambiguous (e.g., change the first TestWithEnvVariable
to TestWithEnvVariable_Correct and the second to TestWithEnvVariable_Incorrect),
and update the surrounding text to call out the rule applies to tests in
*_test.go (tests that use t.Setenv() on a parent must not call t.Parallel());
update references in the examples to match the new identifiers
(TestWithEnvVariable_Correct, TestWithEnvVariable_Incorrect) so copying them
won’t produce duplicate-symbol errors.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 98c68341-5888-45eb-8b82-62458f4488c4
📒 Files selected for processing (19)
.claude/skills/cross-platform-development.claude/skills/implementing-command-patterns.claude/skills/testing-best-practices.claude/skills/testing-commands.claude/skills/working-with-config-system.claude/skills/working-with-instances-manager.claude/skills/working-with-podman-runtime-config.claude/skills/working-with-runtime-system.claude/skills/working-with-steploggerAGENTS.mdskills/cross-platform-development/SKILL.mdskills/implementing-command-patterns/SKILL.mdskills/testing-best-practices/SKILL.mdskills/testing-commands/SKILL.mdskills/working-with-config-system/SKILL.mdskills/working-with-instances-manager/SKILL.mdskills/working-with-podman-runtime-config/SKILL.mdskills/working-with-runtime-system/SKILL.mdskills/working-with-steplogger/SKILL.md
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Updated the skills README to document all 16 available skills, organized into 6 logical categories for easier discovery and navigation by AI agents and developers. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CLAUDE.md was too large (1814 lines) making it difficult to navigate and find specific guidance. Split detailed implementation guides into 9 new skills while keeping essential architectural patterns in core docs. Reduces CLAUDE.md to 348 lines (81% reduction).
New skills created:
Closes #112