-
Notifications
You must be signed in to change notification settings - Fork 667
NO-JIRA: add claude /plugin-api-review command
#15658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@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. |
|
@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. |
|
/verified bypass |
|
@logonoff: The 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. |
|
/meow |
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 kubernetes-sigs/prow repository. |
3791f7e to
b0bfd81
Compare
spadgett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks! 👍
/lgtm
| - CSS classes may be inadvertently used by plugins, so changes to CSS files | ||
| should be reviewed for potential breakages. Perform a search on GitHub for | ||
| the class name to see if it is used in any public plugins. Repos which are | ||
| console dynamic plugins will always consume | ||
| `@openshift-console/dynamic-plugin-sdk` in the `package.json` file of the repo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I assume this will not have false positives on public forks of the openshift/console repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't had any problems with that yet
| - New API or extension points MUST be well documented with descriptions that are | ||
| clear enough to allow plugin authors to understand their purpose and usage without | ||
| referring back to the source code. Assume that plugin authors do not have | ||
| knowledge of the internal workings of OpenShift Console. New documentation SHOULD | ||
| contain examples of usage where applicable, edge cases and behavior when parameters | ||
| such as `null`, `undefined`, or invalid values are provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is important, thank you
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, spadgett The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/verified by @logonoff |
|
@logonoff: This PR has been marked as verified by 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. |
|
/hold to avoid retesting #15679 |
|
/unhold |

example output from #15655
Plugin API Review Report
API change: Internal refactoring to always use canonical model references
JIRA issue: CONSOLE-4837
Current OpenShift version: 4.21
Changes to changelog made? N/A - No plugin API changes
Summary
The changes in this branch are internal console refactoring and do not affect the plugin API. The modified files are:
referenceForModel()instead of conditionally usingpluralorreferenceForModelbased on CRD statususePluralhook (hook version ofconnectToPlural) for internal console useThese files are not part of the
@openshift-console/dynamic-plugin-sdkpackage and are not exported as shared modules or extension APIs. They are internal console implementation files in thefrontend/publicdirectory.Findings
Issues found during review
Code Quality Observations (not plugin API compliance issues):
usePluralfunction callsbuildK8sKind()with the result ofmodelMetadata.find()which can returnundefined. This may cause runtime errors if no matching metadata is found.buildK8sKindwheremetadata.properties?.labelPlural.toLowerCase()doesn't use optional chaining on the.toLowerCase()call.These are internal implementation issues and do not affect plugin API compliance.
Actions taken
None required - no plugin API changes to document.
Compliance score: 10/10
Reason for score: