-
Notifications
You must be signed in to change notification settings - Fork 670
NO-JIRA: Add guidance for avoiding deprecated components #15969
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
base: main
Are you sure you want to change the base?
Conversation
Added deprecation signals to check (JSDoc tags, /deprecated paths, DEPRECATED_ prefix) with code examples in the Patterns section. Assisted by: Claude Code
|
@sg00dwin: 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. |
|
@sg00dwin: all tests passed! 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. |
📝 WalkthroughWalkthroughDocumentation updates to CONVENTIONS.md establishing deprecation guidelines for the Frontend Code Conventions. The new section defines three deprecation signals (JSDoc 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CONVENTIONS.md`:
- Line 94: Replace the bolded section title "**Avoid deprecated components**"
with a level-4 markdown heading (e.g., "#### Avoid deprecated components") so it
matches the document's heading style (see the "TypeScript Type Safety" heading
used earlier); ensure there's a blank line before and after the new heading for
proper rendering.
🧹 Nitpick comments (1)
CONVENTIONS.md (1)
109-112: Reconsider placement of AI agent guidance.The guidance about flagging the
anytype (lines 109-112) is unrelated to deprecated components. It would be more logically organized if:
- Moved to the end of the existing "TypeScript Type Safety" section (after line 69), or
- Placed in a dedicated "AI Agent Guidelines" section
This would improve document navigation and topical coherence.
📚 Suggested reorganization
Option 1: Move after the existing TypeScript Type Safety section:
// BAD - Using 'any' const data: any = fetchData();
+AI agents should flag use of
anytype
+- Suggest proper type definitions
+- Check that null/undefined are handled:string | undefined
+- Verify exported types for reusable componentsThen remove lines 109-112 from under the deprecated components section. </details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ); | ||
| ``` | ||
|
|
||
| **Avoid deprecated components** |
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.
Use proper heading syntax for consistency.
The section title uses bold emphasis (**...**) instead of a heading. For consistency with the document structure (see "TypeScript Type Safety" at line 58), use a level-4 heading.
📝 Proposed fix for markdown heading
-**Avoid deprecated components**
+#### Avoid deprecated components📝 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.
| **Avoid deprecated components** | |
| #### Avoid deprecated components |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In `@CONVENTIONS.md` at line 94, Replace the bolded section title "**Avoid
deprecated components**" with a level-4 markdown heading (e.g., "#### Avoid
deprecated components") so it matches the document's heading style (see the
"TypeScript Type Safety" heading used earlier); ensure there's a blank line
before and after the new heading for proper rendering.
rhamilto
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.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto, sg00dwin 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 |
Summary
@deprecatedtags, import paths containing/deprecated, andDEPRECATED_file name prefix🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.