Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@logonoff: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/label px-approved |
openshift-ci
bot
added
the
px-approved
📝 WalkthroughWalkthroughThis pull request restructures documentation files for the Console Dynamic Plugin SDK. ARCHITECTURE.md is deleted entirely, with its content redistributed across AGENTS.md, STYLEGUIDE.md, and TESTING.md. AGENTS.md gains expanded guidance on Dynamic plugin SDK concepts, public API considerations, and development workflows. TESTING.md transforms from terse bullet lists into structured narrative format with concrete examples for unit, integration, and E2E testing. References to ARCHITECTURE.md are removed from .coderabbit.yaml configuration and SKILL.md guidance references. Minor punctuation adjustments appear in STYLEGUIDE.md. All changes are documentation-focused with no API or code modifications. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 17-18: Update the first lint example to show a targeted file path:
replace the ambiguous "cd frontend && yarn eslint" command with an explicit
placeholder such as "cd frontend && yarn eslint <path/to/file>" so the note
about specifying a file path is consistent, and keep the existing "cd frontend
&& yarn lint" line for full-repo linting; make sure both commands and their
explanatory comments reflect targeted vs full-repo usage.
- Around line 45-58: Tighten the wording in the SDK guidance to remove
ambiguity: rewrite the sentence starting with "In addition Kubernetes cluster
management, OpenShift console is designed..." to "In addition to Kubernetes
cluster management, the OpenShift Console is an extensible platform that
integrates additional UI functionality via operators and custom resources.";
clarify the breaking-changes sentence to "⚠️ BREAKING CHANGES: Exercise extreme
care—this public API is consumed by external plugins."; and rephrase the
re-exports guidance to "Before making changes, verify they do not impact the
public API by checking
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-*.ts files to
avoid breaking external plugins." Reference the token
"console-dynamic-plugin-sdk" and the internal-*.ts path when applying edits.
In `@TESTING.md`:
- Around line 153-155: Replace the non-executable wildcard pseudo-commands in
TESTING.md (the lines showing `yarn test-cypress-[a-z]*`, `yarn
test-cypress-[a-z]*-headless`, and `yarn test-cypress-[a-z]*-nightly`) with
either a clear placeholder like `yarn test-cypress-<suite>`, `yarn
test-cypress-<suite>-headless`, `yarn test-cypress-<suite>-nightly` or by giving
concrete example script names such as `yarn test-cypress-olm`, `yarn
test-cypress-olm-headless`, `yarn test-cypress-olm-nightly`; update the
surrounding text to state that `<suite>` should be replaced with the actual
suite name so readers won’t attempt to run invalid wildcard commands.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
.claude/skills/update-package/SKILL.md.coderabbit.yamlAGENTS.mdARCHITECTURE.mdSTYLEGUIDE.mdTESTING.md
💤 Files with no reviewable changes (2)
- .coderabbit.yaml
- ARCHITECTURE.md
AGENTS.md
Outdated
| cd frontend && yarn eslint # ESLint / prettier linting (must specify file path) | ||
| cd frontend && yarn lint # ESLint / prettier linting (lints entire codebase) |
There was a problem hiding this comment.
Fix command/example mismatch for targeted linting.
On Line 17, the command omits a file path even though the note says one is required. This keeps the guidance ambiguous right where the PR is trying to clarify lint behavior. Use an explicit placeholder example (for example, yarn eslint <path/to/file>), while keeping Line 18 for full-repo linting.
Suggested doc diff
-cd frontend && yarn eslint # ESLint / prettier linting (must specify file path)
+cd frontend && yarn eslint <path/to/file> # ESLint / prettier linting for a specific file
cd frontend && yarn lint # ESLint / prettier linting (lints entire codebase)📝 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.
| cd frontend && yarn eslint # ESLint / prettier linting (must specify file path) | |
| cd frontend && yarn lint # ESLint / prettier linting (lints entire codebase) | |
| cd frontend && yarn eslint <path/to/file> # ESLint / prettier linting for a specific file | |
| cd frontend && yarn lint # ESLint / prettier linting (lints entire codebase) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 17 - 18, Update the first lint example to show a
targeted file path: replace the ambiguous "cd frontend && yarn eslint" command
with an explicit placeholder such as "cd frontend && yarn eslint <path/to/file>"
so the note about specifying a file path is consistent, and keep the existing
"cd frontend && yarn lint" line for full-repo linting; make sure both commands
and their explanatory comments reflect targeted vs full-repo usage.
AGENTS.md
Outdated
| In addition Kubernetes cluster management, OpenShift console is designed to be an extensible platform that integrates with additional UI functionality with operators and custom resources. The `console-dynamic-plugin-sdk` is the public API that enables dynamic plugins to integrate with the console at runtime. | ||
|
|
||
| **⚠️ BREAKING CHANGES REQUIRE EXTREME CARE**: This is a public API consumed by external plugins | ||
|
|
||
| ### Re-exports | ||
|
|
||
| The dynamic plugin SDK provides a re-export layer to allow dynamic plugins to consume APIs from multiple Console packages without directly depending on them. Before starting ANY changes, ensure your change not impact the public API by checking `frontend/packages/console-dynamic-plugin-sdk/src/api/internal-*.ts` files to avoid breakage. | ||
|
|
||
| ### Development guidelines | ||
|
|
||
| - Always consider impact on external plugin developers | ||
| - Maintain backward compatibility as it's a public API | ||
| - Comprehensive documentation for all public APIs | ||
| - Changes to extension schema changes need migration paths |
There was a problem hiding this comment.
Tighten grammar in SDK guidance to avoid ambiguity.
A few phrases are grammatically off and can be misread in critical API-compatibility guidance (Line 45, Line 51, Line 58). Recommend cleaning these for precision.
Suggested doc diff
-In addition Kubernetes cluster management, OpenShift console is designed to be an extensible platform that integrates with additional UI functionality with operators and custom resources.
+In addition to Kubernetes cluster management, OpenShift console is designed to be an extensible platform that integrates additional UI functionality with operators and custom resources.
-Before starting ANY changes, ensure your change not impact the public API by checking ...
+Before starting ANY changes, ensure your changes do not impact the public API by checking ...
-- Changes to extension schema changes need migration paths
+- Changes to extension schemas need migration paths📝 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.
| In addition Kubernetes cluster management, OpenShift console is designed to be an extensible platform that integrates with additional UI functionality with operators and custom resources. The `console-dynamic-plugin-sdk` is the public API that enables dynamic plugins to integrate with the console at runtime. | |
| **⚠️ BREAKING CHANGES REQUIRE EXTREME CARE**: This is a public API consumed by external plugins | |
| ### Re-exports | |
| The dynamic plugin SDK provides a re-export layer to allow dynamic plugins to consume APIs from multiple Console packages without directly depending on them. Before starting ANY changes, ensure your change not impact the public API by checking `frontend/packages/console-dynamic-plugin-sdk/src/api/internal-*.ts` files to avoid breakage. | |
| ### Development guidelines | |
| - Always consider impact on external plugin developers | |
| - Maintain backward compatibility as it's a public API | |
| - Comprehensive documentation for all public APIs | |
| - Changes to extension schema changes need migration paths | |
| In addition to Kubernetes cluster management, OpenShift console is designed to be an extensible platform that integrates additional UI functionality with operators and custom resources. The `console-dynamic-plugin-sdk` is the public API that enables dynamic plugins to integrate with the console at runtime. | |
| **⚠️ BREAKING CHANGES REQUIRE EXTREME CARE**: This is a public API consumed by external plugins | |
| ### Re-exports | |
| The dynamic plugin SDK provides a re-export layer to allow dynamic plugins to consume APIs from multiple Console packages without directly depending on them. Before starting ANY changes, ensure your changes do not impact the public API by checking `frontend/packages/console-dynamic-plugin-sdk/src/api/internal-*.ts` files to avoid breakage. | |
| ### Development guidelines | |
| - Always consider impact on external plugin developers | |
| - Maintain backward compatibility as it's a public API | |
| - Comprehensive documentation for all public APIs | |
| - Changes to extension schemas need migration paths |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 45 - 58, Tighten the wording in the SDK guidance to
remove ambiguity: rewrite the sentence starting with "In addition Kubernetes
cluster management, OpenShift console is designed..." to "In addition to
Kubernetes cluster management, the OpenShift Console is an extensible platform
that integrates additional UI functionality via operators and custom
resources."; clarify the breaking-changes sentence to "⚠️ BREAKING CHANGES:
Exercise extreme care—this public API is consumed by external plugins."; and
rephrase the re-exports guidance to "Before making changes, verify they do not
impact the public API by checking
frontend/packages/console-dynamic-plugin-sdk/src/api/internal-*.ts files to
avoid breaking external plugins." Reference the token
"console-dynamic-plugin-sdk" and the internal-*.ts path when applying edits.
TESTING.md
Outdated
| - `yarn test-cypress-[a-z]*` to open an interactive window for a specific Cypress test suite (e.g., `yarn test-cypress-olm`, `yarn test-cypress-devconsole`). **AI agents may struggle with interactive mode**. | ||
| - `yarn test-cypress-[a-z]*-headless` to run the same suite in headless mode (e.g., `yarn test-cypress-olm-headless`) | ||
| - `yarn test-cypress-[a-z]*-nightly` runs an extended suite of tests in headless mode, intended for CI/nightly runs (e.g., `yarn test-cypress-olm-nightly`) |
There was a problem hiding this comment.
Replace wildcard pseudo-commands with executable script examples.
Lines 153–155 present wildcard patterns as runnable commands. This is likely to fail or confuse users; yarn expects concrete script names. Given this PR’s objective, these should be documented as placeholders (e.g., <suite>) or replaced by explicit examples only.
Suggested doc diff
-- `yarn test-cypress-[a-z]*` to open an interactive window for a specific Cypress test suite (e.g., `yarn test-cypress-olm`, `yarn test-cypress-devconsole`). **AI agents may struggle with interactive mode**.
-- `yarn test-cypress-[a-z]*-headless` to run the same suite in headless mode (e.g., `yarn test-cypress-olm-headless`)
-- `yarn test-cypress-[a-z]*-nightly` runs an extended suite of tests in headless mode, intended for CI/nightly runs (e.g., `yarn test-cypress-olm-nightly`)
+- `yarn test-cypress-<suite>` opens an interactive window for a specific Cypress test suite (e.g., `yarn test-cypress-olm`, `yarn test-cypress-devconsole`). **AI agents may struggle with interactive mode**.
+- `yarn test-cypress-<suite>-headless` runs the same suite in headless mode (e.g., `yarn test-cypress-olm-headless`)
+- `yarn test-cypress-<suite>-nightly` runs an extended headless suite intended for CI/nightly runs (e.g., `yarn test-cypress-olm-nightly`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TESTING.md` around lines 153 - 155, Replace the non-executable wildcard
pseudo-commands in TESTING.md (the lines showing `yarn test-cypress-[a-z]*`,
`yarn test-cypress-[a-z]*-headless`, and `yarn test-cypress-[a-z]*-nightly`)
with either a clear placeholder like `yarn test-cypress-<suite>`, `yarn
test-cypress-<suite>-headless`, `yarn test-cypress-<suite>-nightly` or by giving
concrete example script names such as `yarn test-cypress-olm`, `yarn
test-cypress-olm-headless`, `yarn test-cypress-olm-nightly`; update the
surrounding text to state that `<suite>` should be replaced with the actual
suite name so readers won’t attempt to run invalid wildcard commands.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: logonoff The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
sg00dwin
left a comment
sg00dwin
left a comment
There was a problem hiding this comment.
Also should remove the reference in .coderabbit.yaml
https://github.com/openshift/console/blob/main/.coderabbit.yaml#L79
| The dynamic plugin SDK provides a re-export layer to allow dynamic plugins to consume APIs from multiple Console packages without directly depending on them. | ||
|
|
||
| Before starting ANY changes, ensure your changes do not impact the public API by checking `frontend/packages/console-dynamic-plugin-sdk/src/api/internal-*.ts` files to avoid breakage. | ||
|
|
There was a problem hiding this comment.
Seems useful context to list out the packages for agent.
| - **`@console/shared`** - Dashboard components, UI components, hooks | |
| - **`@console/internal`** - Core UI, editors, hooks, K8s utilities | |
| - **`@console/plugin-sdk`** - Extension system, plugin infrastructure | |
| - **`@console/app`** - Application context | |
| - **`@console/topology`** - Topology components, data transforms, graph views |
| In addition to Kubernetes cluster management, OpenShift console is designed to be an extensible platform that integrates additional UI functionality with operators and custom resources. | ||
|
|
||
| The `console-dynamic-plugin-sdk` is the public API that enables dynamic plugins to integrate with the console at runtime. | ||
|
|
There was a problem hiding this comment.
Is there a reason to not include this information from ARCHITECTURE.md
| For the complete list of console plugins, see `console-app/package.json` dependencies. Static plugins (built-in) may be deprecated or extracted over time. | |
| - **Extension Points System**: 25+ extension types (NavItem, Page, ResourceListPage, DashboardsCard, etc.) | |
| - **Module Federation**: Runtime plugin loading via Webpack Module Federation | |
| - **Type Safety**: Comprehensive TypeScript definitions for all extension points | |
| - **Code References**: Lazy loading with `$codeRef` for performance | |
| ### Official extension types & API documentation | |
| - [Dynamic Plugin SDK documentation](frontend/packages/console-dynamic-plugin-sdk/README.md)- Authoritative for how the SDK works | |
| - [Extension Types Reference](frontend/packages/console-dynamic-plugin-sdk/docs/console-extensions.md) - Complete extension type definitions, naming conventions (`console.*`), and deprecation notices | |
| - [Console API Documentation](frontend/packages/console-dynamic-plugin-sdk/docs/api.md) - React components, hooks, utilities, and TypeScript types exported by the SDK | |
There was a problem hiding this comment.
It's already in the dynamic Plugin SDK documentation. I get your point though so I'll add a synopsis
AGENTS.md
Outdated
| ### Dynamic plugin SDK | ||
|
|
||
| - **[frontend/packages/console-dynamic-plugin-sdk/README.md](frontend/packages/console-dynamic-plugin-sdk/README.md)** - Comprehensive dynamic plugin SDK documentation | ||
| - [Dynamic Plugin SDK documentation](frontend/packages/console-dynamic-plugin-sdk/README.md)- Authoritative for how the SDK works |
There was a problem hiding this comment.
| - [Dynamic Plugin SDK documentation](frontend/packages/console-dynamic-plugin-sdk/README.md)- Authoritative for how the SDK works | |
| - [Dynamic Plugin SDK documentation](frontend/packages/console-dynamic-plugin-sdk/README.md) - Authoritative for how the SDK works |
|
@logonoff: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
|
/label docs-approved |
openshift-ci
bot
added
the
docs-approved
3 participants
Summary by CodeRabbit