Add learning plugin for educational code understanding #97
Conversation
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @ibihim. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
openshift-ci
bot
added
ok-to-test
| @@ -0,0 +1,174 @@ | |||
| --- | |||
| description: Create a Liz Rice-style walkthrough building a concept from first principles with working code | |||
There was a problem hiding this comment.
I think we are not wanting to get AI's to generate any content as if they were a specific human. Especially without permission, but this is probably a general rule we should establish in the repo regardless. Instead, I'd suggest frame it as what qualities you like about how she presents the material. It's also better for the AI to understand
There was a problem hiding this comment.
Ok, makes sense. I just realized, while playing with the AI, that telling the AI to embody a particular person is very effective.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ibihim The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-merge-robot
added
the
needs-rebase
WalkthroughAdds three new plugins (learning, olm, must-gather), extensive per-plugin documentation and analyzer scripts, website generation and documentation tooling, a claudelint rule enforcing generated docs, CI/workflow and Makefile updates, and assorted plugin command docs and README/site files. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant Repo as Repository
participant Gen as scripts/generate_plugin_docs.py
participant Build as scripts/build-website.py
participant Files as PLUGINS.md / docs/data.json
participant Lint as .claudelint-custom PluginsDocUpToDateRule
Dev->>Repo: push / run make update
Note over Repo,Gen: make update invokes generation steps
Repo->>Gen: run generate_plugin_docs.py
Gen-->>Files: write PLUGINS.md
Repo->>Build: run build-website.py
Build-->>Files: write docs/data.json
Lint->>Repo: (on check) executes PluginsDocUpToDateRule
PluginsDocUpToDateRule->>Gen: run generate_plugin_docs.py (sandboxed)
PluginsDocUpToDateRule->>Build: run build-website.py (if present)
PluginsDocUpToDateRule->>Files: compare generated vs committed
alt differences found
PluginsDocUpToDateRule-->>Dev: report violation (restore originals)
else no diffs
PluginsDocUpToDateRule-->>Dev: pass
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (6)
PLUGINS.md (1)
86-94: Address markdown lint violations in Learning Plugin section.The markdownlint tool flagged MD036 (emphasis as heading) on Line 94. While this is a minor style issue, ensure the markdown conforms to the repository's lint rules that are checked in CI (
make lint).The link itself looks fine—ensure the linter passes when running the full check.
plugins/learning/README.md (2)
57-72: Fix markdown linting violations (MD036 and style repetition).The markdownlint tool flagged MD036 violations at lines 57, 63, 69, and 72, where emphasis (bold text in bullet lists) appears before the colon. The linter interprets
**Use /learning:explain when you:**as emphasis-as-heading. Restructure these bullet items to avoid the violation.Apply this diff to resolve the MD036 violations:
-### Use `/learning:explain` when you: - -- Encounter unfamiliar code and need to understand it deeply -- Want to understand design decisions and trade-offs +### Use `/learning:explain` when you + +- Encounter unfamiliar code and need to understand it deeply +- Understand design decisions and trade-offsRepeat similarly for the
/learning:build-from-scratchsection (lines 66–72).
12-12: Specify language identifiers for all fenced code blocks (MD040).Fenced code blocks at lines 12, 97, 103, 109, and 115 lack language specifiers. Add appropriate language tags (e.g.,
bash,shell) to satisfy the markdown linter and improve readability.Examples:
- ``` + ```bash # Add the ai-helpers marketplace /plugin marketplace add openshift-eng/ai-helpersAlso applies to: 97-97, 103-103, 109-109, 115-115
plugins/learning/commands/build-from-scratch.md (1)
12-12: Specify language identifiers for code block examples (MD040).Code block examples at lines 12, 129, 137, 145, and 153 lack language identifiers. Add
bashor appropriate language tags to satisfy the linter and improve syntax highlighting.-``` +```bash /learning:build-from-scratch container runtimeAlso applies to: 129-129, 137-137, 145-145, 153-153
plugins/learning/commands/explain.md (2)
57-57: Fix markdown linting violations (MD036) in WHAT/WHY/HOW section.The markdownlint tool flagged MD036 violations where bold-formatted section markers (
**WHAT:**,**WHY:**,**HOW:**) appear in the structured explanation section. Restructure as proper markdown headings to satisfy linting rules.-**WHAT: High-Level Overview** +#### WHAT: High-Level OverviewApply similar changes to lines 63 and 69.
Also applies to: 63-63, 69-69
12-12: Specify language identifiers for code block examples (MD040).Code block examples lack language identifiers at lines 12, 97, 103, 109, and 115. Add appropriate tags (e.g.,
bash,shell) for linting compliance and syntax highlighting.-``` +```bash /learning:explain pkg/reconciler/controller.goAlso applies to: 97-97, 103-103, 109-109, 115-115
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
.claude-plugin/marketplace.json(1 hunks)PLUGINS.md(2 hunks)docs/data.json(1 hunks)plugins/learning/.claude-plugin/plugin.json(1 hunks)plugins/learning/README.md(1 hunks)plugins/learning/commands/build-from-scratch.md(1 hunks)plugins/learning/commands/explain.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
plugins/learning/README.md
[style] ~63-~63: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... to explain complex systems to others - Want to build mental models of technical concep...
(REP_WANT_TO_VB)
[style] ~69-~69: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...arn best by building working examples - Need to teach concepts to others - Want to demy...
(REP_NEED_TO_VB)
[style] ~70-~70: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...es - Need to teach concepts to others - Want to demystify complex technologies - Prepar...
(REP_WANT_TO_VB)
[style] ~72-~72: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...technical interviews or presentations - Want to contribute to systems but need foundati...
(REP_WANT_TO_VB)
plugins/learning/commands/build-from-scratch.md
[uncategorized] ~76-~76: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...the Walkthrough Create a comprehensive markdown document with: *Introduction Section...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.18.1)
plugins/learning/README.md
12-12: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
57-57: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
63-63: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
69-69: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
97-97: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
plugins/learning/commands/explain.md
12-12: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
57-57: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
63-63: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
69-69: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
97-97: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
115-115: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
PLUGINS.md
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (5)
plugins/learning/.claude-plugin/plugin.json (1)
1-8: Plugin metadata is well-formed and consistent.The plugin descriptor looks good—clean JSON structure, correct versioning, and proper author attribution.
.claude-plugin/marketplace.json (1)
37-41: Marketplace registration is properly integrated.The learning plugin entry is well-structured and correctly positioned in the marketplace plugins list. Description and source path are consistent with the plugin metadata.
plugins/learning/commands/build-from-scratch.md (1)
29-172: Comprehensive and well-structured command documentation.The four-phase implementation plan (analysis, incremental steps, structure, verification) is clear and actionable. Examples are concrete and progressive. The documentation effectively communicates the educational approach and expected output. The integration of WebSearch guidance (line 36) and structured code organization aligns well with the plugin's pedagogical goals.
docs/data.json (1)
214-234: Plugin registry entry is correctly structured and consistent.The learning plugin data is properly formatted with accurate command synopses, argument hints, and metadata. The entry follows the established schema and aligns with the marketplace and plugin metadata definitions.
The AI-generated summary mentions the learning plugin entry appears twice in docs/data.json. Please verify there are no duplicate plugin entries that should be consolidated.
plugins/learning/commands/explain.md (1)
27-87: Well-structured, comprehensive command documentation.The four-phase implementation (Input Analysis, First Principles Analysis, Structured Explanation, Verification and Depth) provides clear guidance on the command's execution flow. The WHAT/WHY/HOW framework is thoroughly explained and aligned with the plugin's educational mission. Examples are practical and cover diverse input types. The inclusion of WebSearch in Phase 1 and rigorous first-principles checking in Phase 4 demonstrates thoughtful design.
It doesn't work, and folks are used to just looking at required statuses on their PR's anyway.
Add comprehensive skills for creating different Jira issue types with templates, best practices, and interactive workflows. Type-Specific Skills: - create-story: User story format, acceptance criteria, 3 Cs framework - create-epic: Epic structure, scope guidelines, parent linking - create-feature: Market problem framework, success criteria, strategic planning - create-task: Technical task structure, task vs story differentiation - create-bug: Bug report template, reproduction steps, severity handling Project-Specific Skills: - cntrlplane: CNTRLPLANE project conventions (stories, epics, features, tasks) - ocpbugs: OCPBUGS project conventions (bugs only) Team-Specific Skills: - hypershift: HyperShift team component selection (ARO/ROSA/HyperShift) Architecture: - No duplication across project/team skills (~1% structural duplication in type skills) - Each skill is self-contained and independently readable - Skills provide implementation guidance for Claude when executing /jira:create command 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add /jira:create command that provides a single interface for creating all Jira issue types (stories, epics, features, tasks, bugs) with intelligent defaults and interactive guidance. Command Features: - Single unified command: /jira:create <type> [project] <summary> [options] - Hybrid workflow: required arguments + optional flags + interactive prompts - Universal requirements automatically applied (Security: Red Hat Employee, Labels: ai-generated-jira) - Security validation: scans for credentials and secrets before submission - Modular skill invocation: automatically loads type, project, and team skills Supported Issue Types: - story: User stories with acceptance criteria - epic: Epics with parent feature linking - feature: Strategic features with market problem analysis - task: Technical tasks and operational work - bug: Bug reports with structured templates Usage Examples: - /jira:create story CNTRLPLANE "Add user dashboard" - /jira:create epic CNTRLPLANE "Mobile redesign" --parent FEATURE-100 - /jira:create bug "Login button doesn't work" - /jira:create feature CNTRLPLANE "Advanced search" Documentation follows man-page format with comprehensive sections for implementation, error handling, examples, and best practices. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This will give indicators of unusual things going on in the cluster at the time an e2e test failed, and provides helpful context when debugging some failures.
This is a first pass idea of some skills for analyzing and extracting basic data (from yamls, no logs yet) from Openshift must-gathers
Update the spec file save location from contrib/ to .work/jira/solve/ for better organization of JIRA solve specification documents. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add new slash command that generates Conventional Commits format messages with two operational modes: - Mode 1: Analyze staged changes and suggest 3 commit messages - Mode 2: Analyze N commits for rewriting or squashing Includes comprehensive documentation with usage examples and type reference.
Add Git plugin section to README with command descriptions.
Signed-off-by: chiragkyal <[email protected]>
Add /git:branch-cleanup command to clean up old and merged branches with support for dry-run, merged-only, and remote deletion modes. Includes safety checks to prevent deletion of protected branches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Adds a comprehensive marketplace website to showcase all plugins, commands, and skills in the ai-helpers repository: - Interactive plugin browser with search functionality - Linkable install instructions and plugin details - Copy-to-clipboard install commands for each plugin - Responsive design with dark theme - Auto-generated from plugin metadata via `make update` - Updated linter to verify docs/data.json stays in sync The website is served from docs/ for GitHub Pages deployment. Co-Authored-By: Claude <[email protected]>
Signed-off-by: chiragkyal <[email protected]>
…atch commands Add new plugin for educational purposes with two commands: - explain: First principles explanations using WHAT/WHY/HOW structure - build-from-scratch: Progressive walkthroughs building systems from fundamentals The plugin helps engineers understand code and concepts through structured learning approaches.
ibihim
force-pushed
the
learning-plugin
branch
from
985521a to
da0fc27
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (34)
plugins/git/commands/branch-cleanup.md (1)
10-12: Add language specification to synopsis code block.For consistency with the repository's markdown standards, consider adding a language specification to the synopsis code block.
Apply this diff:
## Synopsis -``` +```bash /git:branch-cleanup [--dry-run] [--merged-only] [--remote]</blockquote></details> <details> <summary>plugins/openshift/commands/destroy-cluster.md (1)</summary><blockquote> `10-12`: **Add language specification to synopsis code block.** For consistency with repository conventions, add a language specification to the synopsis code block. Apply this diff: ```diff ## Synopsis -``` +```bash /openshift:destroy-cluster [install-dir]</blockquote></details> <details> <summary>plugins/git/commands/commit-suggest.md (1)</summary><blockquote> `10-13`: **Add language specification to synopsis code block.** For consistency with repository markdown standards, add a language specification to the synopsis code block. Apply this diff: ```diff ## Synopsis -``` +```bash /git:commit-suggest # Analyze staged changes /git:commit-suggest [N] # Analyze last N commits (1-100)</blockquote></details> <details> <summary>plugins/prow-job/skills/prow-job-analyze-test-failure/SKILL.md (1)</summary><blockquote> `87-109`: **Clarify wildcard copy behavior and fallback approach.** The wildcard copy on lines 97-99 may not work reliably with `gcloud storage cp` for nested paths, and the note about fallback on line 99 could be clearer. Consider emphasizing the individual file copy approach as the primary method, since the `ls` command on line 91 already provides the full paths. Apply this diff to clarify the approach: ```diff - - Download all matching interval files (use the full paths from the search results): + - Download all matching interval files using the full paths from the search results: ```bash - gcloud storage cp gs://test-platform-results/{bucket-path}/**/e2e-timelines_spyglass_*.json .work/prow-job-analyze-test-failure/{build_id}/logs/ --no-user-output-enabled + # For each file found by the ls command above, download individually: + gcloud storage cp gs://test-platform-results/{full-path-from-search} .work/prow-job-analyze-test-failure/{build_id}/logs/ --no-user-output-enabled ``` - - If the wildcard copy doesn't work, copy each file individually using the full paths from the search resultsplugins/must-gather/skills/must-gather-analyzer/scripts/analyze_clusterversion.py (2)
15-24: Narrow overly broad exception handling in parser and duration helperCatching bare
Exceptioninparse_clusterversionandcalculate_durationmasks unexpected errors and trips BLE001. You only need to handle parse/format issues here.def parse_clusterversion(file_path: Path) -> Optional[Dict[str, Any]]: """Parse the clusterversion YAML file.""" try: with open(file_path, 'r') as f: doc = yaml.safe_load(f) if doc and doc.get('kind') == 'ClusterVersion': return doc - except Exception as e: + except (OSError, yaml.YAMLError) as e: print(f"Warning: Failed to parse {file_path}: {e}", file=sys.stderr) return None @@ def calculate_duration(timestamp_str: str) -> str: """Calculate duration from timestamp to now.""" try: ts = datetime.fromisoformat(timestamp_str.replace('Z', '+00:00')) now = datetime.now(ts.tzinfo) delta = now - ts @@ - else: - return "<1m" - except Exception: - return "" + else: + return "<1m" + except (TypeError, ValueError): + return ""Also applies to: 35-56
159-197: Clean up lint issues: unnecessary f-strings and unused variableA few lines are tripping F541/F841 (f-strings without placeholders and an unused
image).- print(f"\nCONDITIONS:") + print("\nCONDITIONS:") @@ - if history: - print(f"\nUPDATE HISTORY (last 5):") + if history: + print("\nUPDATE HISTORY (last 5):") @@ - if available_updates and isinstance(available_updates, list) and len(available_updates) > 0: - print(f"\nAVAILABLE UPDATES ({len(available_updates)}):") + if available_updates and isinstance(available_updates, list) and len(available_updates) > 0: + print(f"\nAVAILABLE UPDATES ({len(available_updates)}):") for i, update in enumerate(available_updates[:5]): version = update.get('version', 'unknown') - image = update.get('image', '') print(f" {i+1}. {version}") elif available_updates is None: - print(f"\nAVAILABLE UPDATES: Unable to retrieve updates") + print("\nAVAILABLE UPDATES: Unable to retrieve updates")plugins/must-gather/skills/must-gather-analyzer/scripts/analyze_etcd.py (2)
16-31: Remove unusedendpoint_healthparsing to simplify data structure
endpoint_health.jsonis parsed twice (once intomember_health, once intoendpoint_health), but theendpoint_healthkey is never read. This is confusing copy‑paste cruft.def parse_etcd_info(must_gather_path: Path) -> Dict[str, Any]: """Parse etcd_info directory for cluster health information.""" etcd_data = { 'member_health': [], 'member_list': [], - 'endpoint_health': [], - 'endpoint_status': [] + 'endpoint_status': [], } @@ - # Parse endpoint health - endpoint_health_file = etcd_info_dir / "endpoint_health.json" - if endpoint_health_file.exists(): - try: - with open(endpoint_health_file, 'r') as f: - data = json.load(f) - etcd_data['endpoint_health'] = data if isinstance(data, list) else [data] - except Exception as e: - print(f"Warning: Failed to parse endpoint_health.json: {e}", file=sys.stderr) -Also applies to: 55-64
39-41: Tighten exception handling and clean constant f-stringsThe JSON parsing blocks catch bare
Exception, and a few log lines are f-strings with no interpolation, both of which Ruff flags.- except Exception as e: + except (OSError, json.JSONDecodeError) as e: print(f"Warning: Failed to parse endpoint_health.json: {e}", file=sys.stderr) @@ - except Exception as e: + except (OSError, json.JSONDecodeError) as e: print(f"Warning: Failed to parse member_list.json: {e}", file=sys.stderr) @@ - except Exception as e: + except (OSError, json.JSONDecodeError) as e: print(f"Warning: Failed to parse endpoint_health.json: {e}", file=sys.stderr) @@ - except Exception as e: + except (OSError, json.JSONDecodeError) as e: print(f"Warning: Failed to parse endpoint_status.json: {e}", file=sys.stderr) @@ - print(f"\n{'='*80}") - print(f"ETCD CLUSTER SUMMARY") - print(f"{'='*80}") + print(f"\n{'='*80}") + print("ETCD CLUSTER SUMMARY") + print(f"{'='*80}") @@ - if healthy_members < total_members: - print(f" ⚠️ Warning: Not all members are healthy!") - elif healthy_members == total_members and total_members > 0: - print(f" ✅ All members healthy") + if healthy_members < total_members: + print(" ⚠️ Warning: Not all members are healthy!") + elif healthy_members == total_members and total_members > 0: + print(" ✅ All members healthy")Also applies to: 52-54, 62-64, 72-74, 147-155
plugins/must-gather/skills/must-gather-analyzer/scripts/analyze_clusteroperators.py (2)
15-24: Avoid catching bareExceptionin ClusterOperator parsing and duration logicSimilar to the other analyzers, the broad
Exceptioncatches here hide unexpected failures.def parse_clusteroperator(file_path: Path) -> Optional[Dict[str, Any]]: """Parse a single clusteroperator YAML file.""" try: with open(file_path, 'r') as f: doc = yaml.safe_load(f) if doc and doc.get('kind') == 'ClusterOperator': return doc - except Exception as e: + except (OSError, yaml.YAMLError) as e: print(f"Warning: Failed to parse {file_path}: {e}", file=sys.stderr) return None @@ def calculate_duration(timestamp_str: str) -> str: @@ - elif minutes > 0: - return f"{minutes}m" - else: - return "<1m" - except Exception: - return "unknown" + elif minutes > 0: + return f"{minutes}m" + else: + return "<1m" + except (TypeError, ValueError): + return "unknown"Also applies to: 41-62
41-72: Consider sharing duration/condition helpers across analyzer scripts
calculate_duration/get_condition_durationhere are nearly identical to helpers in the ClusterVersion and events analyzers. Extracting them into a small shared utility (e.g.,time_utils.py) would reduce duplication and keep behavior consistent if you tweak the formatting later.plugins/must-gather/skills/must-gather-analyzer/scripts/analyze_pvs.py (1)
15-23: Tighten YAML parse error handling and drop unnecessary f-stringThe generic
except Exceptionand a constant f-string in the summary are low-hanging cleanup that also satisfy BLE001/F541.def parse_yaml_file(file_path: Path) -> Optional[Dict[str, Any]]: """Parse a YAML file.""" try: with open(file_path, 'r') as f: doc = yaml.safe_load(f) return doc - except Exception as e: + except (OSError, yaml.YAMLError) as e: print(f"Warning: Failed to parse {file_path}: {e}", file=sys.stderr) return None @@ - print(f"\n{'='*80}") - print(f"SUMMARY") + print(f"\n{'='*80}") + print("SUMMARY") print(f"PVs: {total_pvs} total ({bound_pvs} bound, {available_pvs} available)") print(f"PVCs: {total_pvcs} total ({bound_pvcs} bound, {pending_pvcs} pending)")Also applies to: 200-203
plugins/must-gather/skills/must-gather-analyzer/scripts/analyze_events.py (2)
17-31: Replace bareExceptioncatches in event parsing and age calculationThe event parser and age helper both use broad
Exception, which can hide unexpected bugs and are flagged by BLE001.def parse_events_file(file_path: Path) -> List[Dict[str, Any]]: @@ - try: - with open(file_path, 'r') as f: - docs = yaml.safe_load_all(f) - for doc in docs: - if doc and doc.get('kind') == 'Event': - events.append(doc) - elif doc and doc.get('kind') == 'EventList': - # Handle EventList - events.extend(doc.get('items', [])) - except Exception as e: + try: + with open(file_path, 'r') as f: + docs = yaml.safe_load_all(f) + for doc in docs: + if doc and doc.get('kind') == 'Event': + events.append(doc) + elif doc and doc.get('kind') == 'EventList': + # Handle EventList + events.extend(doc.get('items', [])) + except (OSError, yaml.YAMLError) as e: print(f"Warning: Failed to parse {file_path}: {e}", file=sys.stderr) @@ def calculate_age(timestamp_str: str) -> str: @@ - elif minutes > 0: - return f"{minutes}m" - else: - return "<1m" - except Exception: - return "" + elif minutes > 0: + return f"{minutes}m" + else: + return "<1m" + except (TypeError, ValueError): + return ""Also applies to: 34-55
59-63: Remove unusednameand avoid ambiguous ℹ characterMinor cleanups to satisfy Ruff: drop the unused
namelocal and replace the ambiguousℹsymbol.def format_event(event: Dict[str, Any]) -> Dict[str, Any]: @@ - metadata = event.get('metadata', {}) - - namespace = metadata.get('namespace', '') - name = metadata.get('name', 'unknown') + metadata = event.get('metadata', {}) + + namespace = metadata.get('namespace', '') @@ - print(f"\nShowing {total} most recent events") - if warnings > 0: - print(f" ⚠️ {warnings} Warning events") - if normal > 0: - print(f" ℹ️ {normal} Normal events") + print(f"\nShowing {total} most recent events") + if warnings > 0: + print(f" ⚠️ {warnings} Warning events") + if normal > 0: + print(f" i {normal} Normal events")Also applies to: 163-167
plugins/must-gather/skills/must-gather-analyzer/SKILL.md (1)
56-206: Consider adding language specifiers to code blocks for better syntax highlighting.The documentation is comprehensive and well-structured. To enhance readability, consider adding
bashlanguage specifiers to the shell command code blocks throughout the file (lines 60, 74, 84, etc.). This enables proper syntax highlighting in markdown renderers.Example for lines 60-62:
-``` +```bash ./scripts/analyze_clusterversion.py <must-gather-path>Also, the bare URL at line 135 could be formatted as a proper markdown link for better rendering. </blockquote></details> <details> <summary>plugins/git/README.md (1)</summary><blockquote> `11-13`: **Consider hyphenating the compound modifier.** The term "Conventional Commits" used as a modifier before "commit messages" should be hyphenated: "Conventional Commits-style commit messages". This follows standard English grammar for compound modifiers and improves clarity. ```diff -Generate Conventional Commits style commit messages for staged changes or recent commits. +Generate Conventional Commits-style commit messages for staged changes or recent commits.Based on static analysis hints.
plugins/learning/README.md (1)
1-125: LGTM! Comprehensive and well-structured plugin documentation.The README provides excellent documentation for the learning plugin with clear explanations of both commands, their use cases, and learning philosophy. The examples and installation instructions are practical and actionable.
Optional improvements for enhanced readability:
Add language specifiers to code blocks (lines 21, 41, 87, 97) for syntax highlighting:
-``` +```bashConsider varying the sentence structure in the "When to Use" sections (lines 59-72) to reduce repetition of "Want to" and "Need to" phrases. For example:
- "Want to understand..." → "You need to understand..."
- "Need to teach..." → "You're teaching..."
The bold text at lines 78 and 94 ("First Principles Thinking" and "Learning by Building") could be converted to proper subheadings (####) for better document structure.
These are minor stylistic improvements and don't affect the quality of the documentation.
plugins/olm/commands/upgrade.md (1)
10-12: Fix markdown linting issues: add language specs to code blocks and wrap bare URLsSeveral instances of:
- MD040: Fenced code blocks missing language specification (e.g., lines 10, 80, 107, 120, 134, 145, 159, 188, 206, 238, 243, 248, 254, 259)
- MD034: Bare URL at line 77 should be wrapped in markdown link syntax
Add bash language identifiers to all bash command examples and wrap the documentation link at line 77 in markdown syntax:
[link text](url).Also applies to: 77-77, 80-80, 107-107, 120-120, 134-134, 145-145, 159-159, 188-188, 206-206, 238-238, 243-243, 248-248, 254-254, 259-259
plugins/olm/README.md (1)
301-301: Address markdown linting: code block language specs and URL wrappingThis comprehensive documentation has good content, but several markdown linting issues should be fixed:
- MD040 (lines 301, 313, 318, 323): Add language identifiers to fenced code blocks (use
bashor appropriate language)- MD034 (lines 506, 533): Wrap bare URLs in markdown link syntax
Also applies to: 313-313, 318-318, 323-323, 506-506, 533-533
plugins/olm/commands/catalog.md (1)
10-10: Markdown linting issues in OLM catalog command documentationGood comprehensive documentation. Fix these markdown linting violations:
- MD040: Code blocks at lines 10, 96, 186, 191, 196 missing language specification (add
bashoryaml)- MD034: Bare URLs at lines 207, 213-217 should be wrapped in markdown link syntax
Also applies to: 96-96, 186-186, 191-191, 196-196, 207-207, 213-217
plugins/openshift/commands/bump-deps.md (1)
12-12: Add language identifiers to fenced code blocks.Multiple code blocks are missing language specifications. For consistency with Markdown best practices and to enable syntax highlighting, add language identifiers (e.g.,
```bash,```output) to each fenced code block throughout the file.Example:
- ``` + ```bash oc get crd -o json - ``` + ```Also applies to: 177-177, 183-183, 209-209, 215-215, 240-240, 246-246, 291-291, 297-297, 312-312, 318-318
plugins/olm/commands/install.md (2)
243-243: Convert bare URL to markdown link format.Line 243 contains a bare URL which should be formatted as a markdown link for consistency and accessibility.
- Reference: https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-approving-operator-upgrades_olm-updating-operators + Reference: [Approving operator upgrades](https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-approving-operator-upgrades_olm-updating-operators)
10-10: Add language identifiers to fenced code blocks.Multiple code blocks are missing language specifications. Add language identifiers (e.g.,
```bash,```yaml) to enable proper syntax highlighting.Also applies to: 113-113, 131-131, 180-180, 190-190, 196-196, 202-202, 208-208, 213-213, 219-219
plugins/olm/commands/diagnose.md (1)
10-10: Add language identifiers to fenced code blocks.Multiple code blocks lack language specifications. Add identifiers (e.g.,
```bash,```json) for syntax highlighting consistency.Also applies to: 69-69, 100-100, 135-135, 169-169, 195-195, 222-222, 245-245, 264-264
plugins/olm/commands/list.md (1)
10-10: Add language identifiers to fenced code blocks.Multiple code blocks are missing language specifications. Add identifiers (e.g.,
```bash) for consistency and syntax highlighting.Also applies to: 80-80, 102-102, 125-125, 130-130
plugins/jira/commands/create.md (1)
10-10: Add language identifiers to fenced code blocks.Multiple code blocks are missing language specifications. Add identifiers (e.g.,
```bash,```json) for syntax highlighting and consistency.Also applies to: 199-199, 213-213, 219-219, 225-225, 231-231, 237-237, 243-243, 249-249, 255-255, 309-309, 329-329, 340-340, 353-353, 367-367, 383-383, 422-422, 428-428, 434-434, 440-440
plugins/olm/commands/status.md (2)
77-77: Convert bare URL to markdown link.Line 77 contains a bare URL. Format it as a markdown link for better accessibility and consistency.
- Reference: https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-approving-operator-upgrades_olm-updating-operators + Reference: [Approving operator upgrades](https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-approving-operator-upgrades_olm-updating-operators)
10-10: Add language identifiers to fenced code blocks.Multiple code blocks are missing language specifications. Add identifiers (e.g.,
```bash,```yaml) for syntax highlighting.Also applies to: 80-80, 107-107, 120-120, 134-134, 145-145, 159-159, 188-188, 206-206, 238-238, 243-243, 248-248, 254-254, 259-259
plugins/olm/commands/approve.md (2)
301-303: Convert bare URLs to markdown links.Lines 301-303 in the "Additional Resources" section contain bare URLs. Format them as markdown links for consistency and better accessibility.
- - [Red Hat OpenShift: Approving Operator Upgrades](https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-approving-operator-upgrades_olm-updating-operators) - - [Red Hat OpenShift: Updating Installed Operators](https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-updating-operators) - - [Operator Lifecycle Manager Documentation](https://olm.operatorframework.io/) + - [Red Hat OpenShift: Approving Operator Upgrades](https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-approving-operator-upgrades_olm-updating-operators) + - [Red Hat OpenShift: Updating Installed Operators](https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/operators/administrator-tasks#olm-updating-operators) + - [Operator Lifecycle Manager Documentation](https://olm.operatorframework.io/)
10-10: Add language identifiers to fenced code blocks.Multiple code blocks lack language specifications. Add identifiers (e.g.,
```bash,```output) for syntax highlighting and consistency.Also applies to: 96-96, 186-186, 191-191, 196-196
docs/index.html (2)
595-626: Use textContent instead of innerHTML for untrusted plugin metadata.While the plugin data originates from a generated
data.jsonfile (lower risk than user input), usinginnerHTMLwith dynamic plugin names and descriptions could allow HTML injection if the data source is ever compromised or modified. For defense-in-depth, usetextContentfor user-facing strings.Current approach (lines 604, 608, etc.):
<div class="plugin-name">${plugin.name}</div>Consider using DOM methods with textContent, or ensure all plugin data is HTML-escaped:
const nameEl = document.createElement('div'); nameEl.className = 'plugin-name'; nameEl.textContent = plugin.name; // Automatically escapedAlternatively, if continuing with template literals, explicitly escape HTML entities in plugin data during generation.
Also applies to: 680-701
564-581: Add error handling and data validation in loadData().The
loadData()function fetches and processes thedata.jsonfile but lacks validation of the expected data structure. Ifdata.jsonis malformed or missing thepluginskey, this could cause runtime errors.async function loadData() { try { const response = await fetch('data.json'); if (!response.ok) { throw new Error(`Failed to load data: ${response.status}`); } const data = await response.json(); + // Validate data structure + if (!Array.isArray(data.plugins)) { + throw new Error('Invalid data format: plugins is not an array'); + } allPlugins = data.plugins; // ... rest of function } catch (error) { console.error('Error loading data:', error); + // Display error to user + document.getElementById('plugins-grid').innerHTML = + '<div class="no-results show">Failed to load plugins marketplace</div>'; } }plugins/jira/skills/create-story/SKILL.md (1)
43-472: Tighten markdown style: add code-fence languages and avoid bold-only pseudo-headingsContent looks solid; the main issues are stylistic and caught by markdownlint:
- Many fenced code blocks (examples, prompts, Jira templates) are missing a language identifier (e.g., lines around the user story examples, AC formats, prompts, and Jira markup). Adding
text,bash,yaml, orpythonas appropriate will satisfy MD040 and improve readability.- A few lines use bold text as de facto headings (e.g., “Approach 1: Guided Questions”, “Approach 2: Template Assistance”, “Approach 3: Free-Form”). Converting these to proper headings (
###/####) will address MD036 and make the structure more scannable.These are non-functional but worth fixing since you already run markdownlint in CI.
scripts/generate_plugin_docs.py (1)
17-33: Make README lookup robust by tracking plugin path inPluginInfoRight now the README link is derived as
plugins/{plugin.name}/README.md, whileplugin.namecomes fromplugin.json. If a plugin ever uses a display name that doesn’t exactly match its directory name, the README link in PLUGINS.md will break.You can make this robust (and simplify lookup) by storing the plugin path in
PluginInfoand using it when generating docs:-from pathlib import Path -from typing import Dict, List, Tuple +from pathlib import Path +from typing import Dict, List, Tuple @@ -class PluginInfo: - """Information about a plugin.""" - - def __init__(self, name: str, description: str, version: str): - self.name = name - self.description = description - self.version = version - self.commands = [] +class PluginInfo: + """Information about a plugin.""" + + def __init__(self, name: str, description: str, version: str, path: Path): + self.name = name + self.description = description + self.version = version + self.path = path + self.commands: List[Dict[str, str]] = [] @@ - plugin_info = PluginInfo( - name=plugin_data.get('name', plugin_dir.name), - description=plugin_data.get('description', ''), - version=plugin_data.get('version', '0.0.0') - ) + plugin_info = PluginInfo( + name=plugin_data.get('name', plugin_dir.name), + description=plugin_data.get('description', ''), + version=plugin_data.get('version', '0.0.0'), + path=plugin_dir, + ) @@ - # Link to plugin README if it exists - readme_path = plugins_dir / plugin.name / 'README.md' + # Link to plugin README if it exists + readme_path = plugin.path / 'README.md'This keeps PLUGINS.md generation correct even if plugin names and folder names diverge later.
Also applies to: 54-89, 92-146
.claudelint-custom.py (1)
17-31: Align rule messaging with what it actually runs, and consider tightening subprocess/error handlingFunctionally this rule does the right thing, but a couple of details could be clearer:
Message vs. behavior mismatch
- Docstring and
description()say “run 'make update'”, and the error message at Line 66 reports"'make update' failed", but the rule actually runs:
python3 scripts/generate_plugin_docs.py- and optionally
python3 scripts/build-website.py- To avoid confusion for contributors, either:
- Change the subprocess call to invoke your
make updatetarget, or- Update the docstring and messages to say “docs generation scripts” instead of “'make update'”.
For example:
return "PLUGINS.md and docs/data.json must be up-to-date with plugin metadata. Run 'make update' to regenerate."
return "PLUGINS.md and docs/data.json must be up-to-date with plugin metadata. Run 'make update' (or the docs generation scripts) to regenerate."@@
f"'make update' failed: {result.stderr}",
f"generate_plugin_docs.py failed: {result.stderr}",
- Subprocess and exception handling (nice-to-have)
- Using
"python3"relies on PATH; for portability you could usesys.executableinstead.- The broad
except Exceptionis understandable (you don’t want claudelint to crash), but if you want to satisfy BLE001 you could narrow this toOSError | subprocess.SubprocessErrorwhile still turning unexpected issues into a violation.None of this blocks functionality, but aligning messages with behavior will make this rule less surprising to future maintainers.
Also applies to: 49-91, 120-133
| - [Utils](#utils-plugin) | ||
| - [Yaml](#yaml-plugin) | ||
|
|
||
| ### Agendas Plugin |
There was a problem hiding this comment.
Fix heading levels and spelling in centralized plugin documentation
Two issues to address:
-
MD001 - Heading Level: Line 20 and all subsequent plugin section headings use
###(h3) but should use##(h2) to properly nest under the document title. This creates an improper heading hierarchy. -
Spelling: Line 30 contains "Miscellaenous" which should be "Miscellaneous".
Apply this diff to fix both issues:
-### Agendas Plugin
+## Agendas Plugin
A plugin to create various meeting agendas
**Commands:**
- **`/agendas:outcome-refinement`** - Analyze the list of JIRA outcome issues to prepare an outcome refinement meeting agenda.
See [plugins/agendas/README.md](plugins/agendas/README.md) for detailed documentation.
-### Ci Plugin
+## Ci Plugin
-Miscellaenous tools for working with OpenShift CI
+Miscellaneous tools for working with OpenShift CINote: Apply the heading level change to all 13 plugin sections (lines 20, 29, 42, 51, 64, 73, 86, 96, 105, 123, 136, 147, 156, 168).
Also applies to: 30-30
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
20-20: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
In PLUGINS.md around lines 20-168 (specifically lines 20, 29, 30, 42, 51, 64,
73, 86, 96, 105, 123, 136, 147, 156, 168), update each plugin section heading
from ### to ## to correct the heading hierarchy (MD001) and correct the spelling
at line 30 from "Miscellaenous" to "Miscellaneous"; apply the heading change to
all 13 plugin section lines listed and save the file.
| { | ||
| "name": "must-gather", | ||
| "description": "A plugin to analyze and report on must-gather data", | ||
| "version": "0.0.1", | ||
| "author": { | ||
| "name": "openshift" | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify author field naming convention against other plugins.
The AI-generated summary indicates the author name should be "github.com/openshift-eng", but the actual file contains "openshift". Please confirm this aligns with the author naming convention used in other plugin.json files in the repository.
🏁 Script executed:
#!/bin/bash
# Description: Check author naming convention across all plugin.json files
# Find all plugin.json files and extract author.name values
fd -e plugin.json -x grep -A1 '"author"' {} + | grep -A1 '"name"'
# Also check if there's documentation or examples specifying the expected format
find . -name "*.md" -o -name "*.json" | xargs grep -l "author.*openshift" 2>/dev/null | head -10Length of output: 43
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all plugin.json files and display their full contents
echo "=== All plugin.json files found ==="
fd -type f -name 'plugin.json'
echo -e "\n=== Contents of each plugin.json ==="
fd -type f -name 'plugin.json' -x echo "File: {}" \; -x cat {}
# Also search for any author-related patterns in JSON/config files
echo -e "\n=== Files mentioning author in .json files ==="
fd -type f -name '*.json' -x grep -l '"author"' {}
Length of output: 634
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all plugin.json files and display their full contents
echo "=== All plugin.json files found ==="
fd -t f -name 'plugin.json'
echo -e "\n=== Contents of each plugin.json ==="
fd -t f -name 'plugin.json' -exec sh -c 'echo "File: $1"; cat "$1"' _ {} \;
# Also search for any author field in JSON files
echo -e "\n=== All JSON files with author field ==="
fd -t f -name '*.json' -exec grep -l '"author"' {} \;
Length of output: 808
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all plugin.json files using find command
echo "=== All plugin.json files found ==="
find . -name 'plugin.json' -type f
echo -e "\n=== Contents of each plugin.json ==="
find . -name 'plugin.json' -type f -exec sh -c 'echo "File: $1"; cat "$1"' _ {} \;
echo -e "\n=== All JSON files with author field ==="
find . -name '*.json' -type f -exec grep -l '"author"' {} \;
Length of output: 4683
Align author field with repository convention: change "openshift" to "github.com/openshift-eng".
The repository establishes a clear pattern where OpenShift team plugins use "github.com/openshift-eng" as the author name (agendas, git, jira, olm, openshift, utils, learning). The must-gather plugin currently uses "openshift", which diverges from this convention. Update line 6 to maintain consistency.
"author": {
"name": "github.com/openshift-eng"
}🤖 Prompt for AI Agents
In plugins/must-gather/.claude-plugin/plugin.json around lines 1 to 8, the
author.name value is "openshift" which deviates from repository convention;
update the author.name field to "github.com/openshift-eng" so it matches other
OpenShift team plugins and keep formatting valid JSON.
| def print_member_health(members: List[Dict[str, Any]]): | ||
| """Print etcd member health status.""" | ||
| if not members: | ||
| print("No member health data found.") | ||
| return | ||
|
|
||
| print("ETCD MEMBER HEALTH") | ||
| print(f"{'ENDPOINT':<60} {'HEALTH':<10} {'TOOK':<10} ERROR") | ||
|
|
||
| for member in members: | ||
| endpoint = member.get('endpoint', 'unknown')[:60] | ||
| health = 'true' if member.get('health') else 'false' | ||
| took = member.get('took', '') | ||
| error = member.get('error', '') | ||
|
|
||
| print(f"{endpoint:<60} {health:<10} {took:<10} {error}") | ||
|
|
There was a problem hiding this comment.
Fix etcd health evaluation to avoid misclassifying unhealthy members
Using m.get('health') directly in truthiness checks will treat any non‑empty string (including "false") as healthy, skewing the summary and member health display.
def print_member_health(members: List[Dict[str, Any]]):
@@
for member in members:
endpoint = member.get('endpoint', 'unknown')[:60]
- health = 'true' if member.get('health') else 'false'
+ raw_health = member.get('health')
+ health = 'true' if str(raw_health).lower() == 'true' else 'false'
took = member.get('took', '')
error = member.get('error', '')
@@
def print_summary(etcd_data: Dict[str, Any]):
@@
- total_members = len(member_list)
- healthy_members = sum(1 for m in member_health if m.get('health'))
+ total_members = len(member_list)
+ healthy_members = sum(
+ 1 for m in member_health
+ if str(m.get('health')).lower() == 'true'
+ )Also applies to: 138-151
🤖 Prompt for AI Agents
In plugins/must-gather/skills/must-gather-analyzer/scripts/analyze_etcd.py
around lines 78-94 (and similarly for 138-151), the code uses
member.get('health') directly which treats non-empty strings like "false" as
truthy; update the health evaluation to normalize and explicitly compare the
value to boolean True or the string "true" (case-insensitive) so that "false" is
treated as unhealthy, then use that normalized boolean to set the displayed
"true"/"false" string and any summary counts; update all places that rely on
truthiness to use this normalized check.
| def get_pod_status(pod: Dict[str, Any]) -> Dict[str, Any]: | ||
| """Extract pod status information.""" | ||
| metadata = pod.get('metadata', {}) | ||
| status = pod.get('status', {}) | ||
| spec = pod.get('spec', {}) | ||
|
|
||
| name = metadata.get('name', 'unknown') | ||
| namespace = metadata.get('namespace', 'unknown') | ||
| creation_time = metadata.get('creationTimestamp', '') | ||
|
|
||
| # Get container statuses | ||
| container_statuses = status.get('containerStatuses', []) | ||
| init_container_statuses = status.get('initContainerStatuses', []) | ||
|
|
||
| # Calculate ready containers | ||
| total_containers = len(spec.get('containers', [])) | ||
| ready_containers = sum(1 for cs in container_statuses if cs.get('ready', False)) | ||
|
|
||
| # Get overall phase | ||
| phase = status.get('phase', 'Unknown') | ||
|
|
||
| # Determine more specific status | ||
| pod_status = phase | ||
| reason = status.get('reason', '') | ||
|
|
||
| # Check for specific container states | ||
| for cs in container_statuses: | ||
| state = cs.get('state', {}) | ||
| if 'waiting' in state: | ||
| waiting = state['waiting'] | ||
| pod_status = waiting.get('reason', 'Waiting') | ||
| elif 'terminated' in state: | ||
| terminated = state['terminated'] | ||
| if terminated.get('exitCode', 0) != 0: | ||
| pod_status = terminated.get('reason', 'Error') | ||
|
|
||
| # Check init containers | ||
| for ics in init_container_statuses: | ||
| state = ics.get('state', {}) | ||
| if 'waiting' in state: | ||
| waiting = state['waiting'] | ||
| if waiting.get('reason') in ['CrashLoopBackOff', 'ImagePullBackOff', 'ErrImagePull']: | ||
| pod_status = f"Init:{waiting.get('reason', 'Waiting')}" | ||
|
|
||
| # Calculate total restarts | ||
| total_restarts = sum(cs.get('restartCount', 0) for cs in container_statuses) | ||
|
|
||
| # Calculate age | ||
| age = calculate_age(creation_time) if creation_time else '' | ||
|
|
||
| return { | ||
| 'namespace': namespace, | ||
| 'name': name, | ||
| 'ready': f"{ready_containers}/{total_containers}", | ||
| 'status': pod_status, | ||
| 'restarts': str(total_restarts), | ||
| 'age': age, | ||
| 'node': spec.get('nodeName', ''), | ||
| 'is_problem': pod_status not in ['Running', 'Succeeded', 'Completed'] or total_restarts > 0 | ||
| } |
There was a problem hiding this comment.
Remove unused variable.
Line 74 assigns reason but never uses it. This variable can be safely removed.
Apply this diff:
# Get overall phase
phase = status.get('phase', 'Unknown')
# Determine more specific status
pod_status = phase
- reason = status.get('reason', '')
# Check for specific container states
for cs in container_statuses:🧰 Tools
🪛 Ruff (0.14.4)
74-74: Local variable reason is assigned to but never used
Remove assignment to unused variable reason
(F841)
🤖 Prompt for AI Agents
In plugins/must-gather/skills/must-gather-analyzer/scripts/analyze_pods.py
around lines 51 to 110, remove the unused local variable `reason` assigned from
status.get('reason', '') on line 74; it is never referenced elsewhere, so delete
that assignment to avoid an unused-variable warning and keep the rest of the
function unchanged.
| - If Manual and not approved, display message: | ||
| ``` | ||
| ⏸️ InstallPlan created but requires manual approval |
There was a problem hiding this comment.
Fix malformed markdown code fence on line 132.
The code block closing on line 132 has only two backticks instead of three, which breaks the markdown fence. This causes the example output to not render correctly.
2. **Install cert-manager-operator with custom namespace**:
```
/olm:install openshift-cert-manager-operator my-cert-manager
- ``
+ ```
This will install the operator in the `my-cert-manager` namespace.Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
131-131: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In plugins/olm/commands/install.md around lines 130 to 132, the closing markdown
code fence is malformed (two backticks) causing the code block to not render;
replace the two backticks on line 132 with a proper three-backtick fence to
correctly close the code block so the example output renders as intended.
| ``` | ||
| /olm:list cert-manager-operator | ||
| `` |
There was a problem hiding this comment.
Fix malformed markdown code fence on line 132.
Line 132 has only two backticks instead of three, breaking the code block closure. This prevents the example from rendering correctly.
2. **List operators in a specific namespace**:
```
/olm:list cert-manager-operator
- ``
+ ```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| /olm:list cert-manager-operator | |
| `` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In plugins/olm/commands/list.md around lines 130 to 132 the closing markdown
code fence is malformed (only two backticks) which breaks the code block;
replace the two backticks on line 132 with three backticks to properly close the
fenced code block so the example renders correctly.
| ```bash | ||
| # Parse version from image tag (e.g., "4.21.0-ec.2" or "4.21.0-0.ci-2025-10-27-031915") | ||
| VERSION=$(echo "$RELEASE_IMAGE" | grep -oE '[0-9]+\.[0-9]+\.[0-9]+[^"]*' | head -1) | ||
| ``` | ||
|
|
||
| ### 3. Determine Installer Location and Extract if Needed | ||
|
|
||
| ```bash | ||
| INSTALLER_DIR="${installer-dir:-$HOME/.openshift-installers}" | ||
| INSTALLER_PATH="$INSTALLER_DIR/openshift-install-${VERSION}" | ||
| ``` | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Fix invalid shell variable and tighten markdown examples
-
Shell correctness (should be fixed before merge)
The snippet that derivesINSTALLER_DIRuses an invalid shell variable name:INSTALLER_DIR="${installer-dir:-$HOME/.openshift-installers}"Shell variable names can’t contain
-, so this won’t run as written. If the intent is “use the$INSTALLER_DIRenv/flag if set, otherwise default”, it should look like:
-INSTALLER_DIR="${installer-dir:-$HOME/.openshift-installers}"
+INSTALLER_DIR="${INSTALLER_DIR:-$HOME/.openshift-installers}"
Same for later references like `${cluster-name}` / `${base-domain}` in summary examples – for shell they should be `${CLUSTER_NAME}` / `${BASE_DOMAIN}`, or clearly marked as placeholders rather than literal env vars.
2. **Optional markdown polish (lint-driven)**
- Several fenced blocks (Synopsis, example commands, summary snippets) lack a language identifier; adding `bash`, `yaml`, or `text` where appropriate will silence MD040 and help with highlighting.
- Bare URLs (e.g., oc client download, pull-secret links) can be wrapped as `[text](URL)` if you want to satisfy MD034, but this is cosmetic.
The main blocker here is the invalid variable name; the rest is documentation polish.
Also applies to: 214-299, 404-428
---
I'll verify the shell variable naming rules and check the actual file content.
🏁 Script executed:
#!/bin/bash
# Test whether shell variable names can contain hyphens
bash -c 'installer-dir="test" 2>&1' || trueLength of output: 51
🏁 Script executed:
#!/bin/bash
# Test valid shell variable with underscore vs hyphen
bash -c 'INSTALLER_DIR="test"; echo "$INSTALLER_DIR"'
bash -c 'installer_dir="test"; echo "$installer_dir"'
bash -c '${installer-dir:-default}' 2>&1 || trueLength of output: 55
🏁 Script executed:
# Check if the file exists and show content around the specified lines
fd "create-cluster.md" --type fLength of output: 44
🏁 Script executed:
# Show lines 146-157, 214-299, 404-428 from the file if it exists
if [ -f "plugins/openshift/commands/create-cluster.md" ]; then
echo "=== Lines 146-157 ==="; sed -n '146,157p' plugins/openshift/commands/create-cluster.md
echo -e "\n=== Lines 214-299 ==="; sed -n '214,299p' plugins/openshift/commands/create-cluster.md
echo -e "\n=== Lines 404-428 ==="; sed -n '404,428p' plugins/openshift/commands/create-cluster.md
else
echo "File not found"
fiLength of output: 3574
🌐 Web query:
bash shell variable naming rules allowed characters
💡 Result:
Short answer: bash variable names may contain only letters (A–Z, a–z), digits (0–9) and underscore (_); they must begin with a letter or underscore (not a digit) and are case‑sensitive. Use ${name} when you need to disambiguate following characters. [1][2][3]
Sources:
- Bash Reference Manual (definition of Name). [1]
- POSIX / The Open Group (shell name rules). [2]
- Explanation of parser behavior and allowed characters. [3]
Fix invalid shell variable names with hyphens across documentation
The review's core finding is verified: shell variable names cannot contain hyphens, and the documented snippets will fail when copied.
Confirmed issues:
- Line 146–157:
${installer-dir:-...}is invalid; must be${INSTALLER_DIR:-...}or similar (underscore, not hyphen). - Line 214–299:
${cluster-name}-install-...has the same pattern and will fail identically.
Both snippets will immediately error with "command not found" when users attempt to run them. The proposed fix (use underscore in variable names) is correct and necessary.
🤖 Prompt for AI Agents
In plugins/openshift/commands/create-cluster.md around lines 146–157 (and also
update the similar occurrences in lines 214–299), the shell examples use invalid
variable names containing hyphens (e.g., installer-dir, cluster-name) which will
cause "command not found" errors; change those variable names to valid
identifiers with underscores (e.g., INSTALLER_DIR, CLUSTER_NAME), update all
template instances and parameter expansions to use the new names (including
defaulting forms like ${INSTALLER_DIR:-...} and ${CLUSTER_NAME}-install-...),
and ensure consistency/casing across the whole document so copied snippets run
correctly in POSIX shells.
13 participants
Summary
Add new learning plugin with two commands for helping engineers understand code and concepts through structured educational approaches.
Commands
/learning:explain <file|package|concept>Provides deep, first-principles explanations using WHAT/WHY/HOW structure:
Works with:
pkg/reconciler/controller.go)pkg/cache)eventual consistency)/learning:build-from-scratch <topic or technology>Creates Liz Rice-style "Build Your Own X" walkthroughs:
Output in
.work/learning-build-from-scratch/{topic}/Changes
plugins/learning/directory with plugin structureTesting
make lint(all checks passed)