CONSOLE-4778: Bump i18next and react-i18next to the reasonably recent version #15875
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@krishagarwal278: This pull request references CONSOLE-4778 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
openshift-merge-robot
added
the
needs-rebase
|
/label docs-approved |
WalkthroughIntroduce a broad ConsoleTFunction type, migrate many translation function parameters to it or relax explicit TFunction imports, wrap numerous returned translation strings in React fragments, update i18n dependency versions and resolutions, and adjust several locale JSON keys. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
openshift-ci
bot
added
component/metal3
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
73-77: Missing translation function call on line 76.Line 76 returns a raw translation key string without calling the
t()function, unlike all other branches that properly callt().🔎 Proposed fix
const syncErr = getPrometheusMetricValue(prometheusResult, 'SyncError'); if (toInteger(syncErr?.[1])) { // TODO: Add timestamp to the message - return { state: HealthState.WARNING, message: 'vsphere-plugin~Synchronization failed' }; + return { state: HealthState.WARNING, message: t('vsphere-plugin~Synchronization failed') }; }
🧹 Nitpick comments (1)
frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx (1)
21-21: Consider the implications of using a custom TranslationFunction type.The PR introduces a custom
TranslationFunctiontype ((key: string, options?: Record<string, unknown>) => string) instead of using react-i18next'sTFunction. While this simplifies the type signature, it may lose type safety features thatTFunctionprovides, such as namespace validation or translation key checking. This is acceptable if it's necessary for compatibility with the new library versions, but ensure this trade-off is intentional.Based on the library upgrade context in this PR.
Also applies to: 50-50
📜 Review details
Configuration used: Organization 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 ignored due to path filters (2)
dynamic-demo-plugin/yarn.lockis excluded by!**/yarn.lock,!**/*.lockfrontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
dynamic-demo-plugin/package.jsondynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsxfrontend/package.jsonfrontend/packages/console-app/src/components/nodes/NodesPage.tsxfrontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/console-plugin-sdk/src/utils/useTranslationExt.tsfrontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.tsfrontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsxfrontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.tsfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/packages/metal3-plugin/src/validations/validations.tsxfrontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsxfrontend/packages/operator-lifecycle-manager/src/status/csv-status.tsfrontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsxfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxfrontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsxfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/vsphere-plugin/src/components/utils.tsfrontend/public/components/about-modal.tsxfrontend/public/components/cron-job.tsxfrontend/public/components/custom-resource-definition.tsxfrontend/public/components/error.tsxfrontend/public/components/modals/column-management-modal.tsxfrontend/public/components/modals/configure-count-modal.tsxfrontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsxfrontend/public/components/pod.tsxfrontend/public/module/k8s/get-resources.ts
💤 Files with no reviewable changes (1)
- dynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/package.jsonfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/public/components/pod.tsxfrontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsxfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/public/components/modals/configure-count-modal.tsxfrontend/public/components/modals/column-management-modal.tsxfrontend/packages/console-plugin-sdk/src/utils/useTranslationExt.tsfrontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsxfrontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.tsfrontend/packages/metal3-plugin/src/validations/validations.tsxfrontend/packages/console-app/src/components/nodes/NodesPage.tsxfrontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/packages/vsphere-plugin/src/components/utils.tsfrontend/public/components/error.tsxfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxdynamic-demo-plugin/package.jsonfrontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/operator-lifecycle-manager/src/status/csv-status.tsfrontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.tsfrontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsxfrontend/public/module/k8s/get-resources.tsfrontend/public/components/cron-job.tsxfrontend/public/components/about-modal.tsxfrontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsxfrontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsxfrontend/public/components/custom-resource-definition.tsx
🧬 Code graph analysis (16)
frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (2)
Extension(12-12)Extension(29-33)frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/modals/configure-count-modal.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/modals/column-management-modal.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/console-app/src/components/nodes/NodesPage.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/utils.ts (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/public/components/error.tsx (3)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/public/components/namespace.jsx (1)
error(857-857)frontend/public/components/events.jsx (1)
error(386-386)
frontend/packages/vsphere-plugin/src/components/persist.ts (2)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/cron-job.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/public/components/about-modal.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsx (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
🔇 Additional comments (24)
frontend/packages/console-app/src/components/nodes/NodesPage.tsx (1)
272-277: LGTM! Translation wrapped in fragment for type consistency.The change correctly wraps the translation result in a React fragment, ensuring consistent React node output. This aligns with the i18next/react-i18next version update and maintains the same functional behavior while providing better type safety.
frontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsx (1)
2-2: LGTM! Clean migration to import TFunction directly from i18next.Importing the TFunction type directly from i18next rather than the react-i18next re-export is correct and aligns with the PR's objective to update these dependencies.
frontend/public/components/modals/configure-count-modal.tsx (1)
73-76: LGTM! Type-safe string coercion for updated i18next.The explicit
String()coercion ensuresresourceKindsremains a string type, aligning with theRecord<string, string>type expectation formessageVariables. This is a sound defensive practice given the updated react-i18next version (15.1.4), which may return broader types from the translation function.frontend/public/module/k8s/get-resources.ts (2)
3-3: LGTM: Import simplified to align with updated i18next usage.The removal of the
TFunctiontype import is consistent with the PR's objective to update i18next and simplify type dependencies.
106-107: No type compatibility issues after removing the TFunction annotation.The
translateExtensionfunction explicitly acceptsTFunction | ((key: string) => string)as its second parameter (line 32 of extension-i18n.ts). The inferred type oftcbas(value: string) => stringmatches this union type and is fully compatible. The removal of the explicit annotation is type-safe.Likely an incorrect or invalid review comment.
frontend/packages/metal3-plugin/src/validations/validations.tsx (1)
1-1: This import change is consistent with the established codebase pattern and poses no type compatibility issues.The import from
i18nextis used across 20+ files throughout the codebase, andTFunction<'metal3-plugin'>is working correctly in this file. Type compatibility with i18next v23.16.8 is confirmed.frontend/packages/operator-lifecycle-manager/src/status/csv-status.ts (1)
44-59: No actionable issues found.The code correctly uses
ClusterServiceVersionStatusenum values, and the single consumer (getOperatorStatus) is properly designed to handle these enum-based status values with appropriate switch logic. There is no breaking change—the consumer is not expecting translated strings in the status field and is already correctly mapping enum values toHealthStatefor downstream processing.Likely an incorrect or invalid review comment.
frontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.ts (1)
35-36: LGTM! Test mock updated to align with production code type change.The indirection through
mockFnproperly accommodates the widenedtparameter type inimport-submit-utils.ts(line 773), where it changed fromTFunctiontoTFunction | ((key: string) => string).frontend/public/components/custom-resource-definition.tsx (1)
281-284: LGTM! Fragment wrapper aligns with PR-wide pattern.Wrapping the translated conditional in a React fragment changes the return type from
stringtoReactNodewhile preserving the rendered output. This aligns with the broader pattern adopted throughout the codebase for translation outputs.frontend/public/components/modals/column-management-modal.tsx (1)
67-70: LGTM! Fragment wrapper consistent with PR pattern.The fragment wrapper changes the return type from
stringtoReactNodewhile preserving the displayed text. This is consistent with the translation output pattern adopted throughout the codebase.frontend/public/components/about-modal.tsx (1)
56-62: LGTM! Fragment wrapper maintains consistency.The fragment wrapper around the fallback text aligns with the PR-wide pattern for translation outputs, changing the return type from
stringtoReactNodewhile preserving functionality.frontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsx (1)
161-191: LGTM! All return paths consistently use fragment wrappers.All switch cases and the default fallback now return React fragments wrapping the translated text. This maintains consistency with the PR-wide pattern while preserving all functionality and display text.
frontend/public/components/cron-job.tsx (1)
88-91: LGTM! Fragment wrapper consistent with PR pattern.The fragment wrapper around the conditional translation maintains the pattern adopted throughout the codebase, changing the return type from
stringtoReactNodewhile preserving the boolean display logic.frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
772-780: No issues identified. The type widening of thetparameter fromTFunctiontoTFunction | ((key: string) => string)is backward compatible. All callers passtfrom theuseTranslation()hook (which returnsTFunction), and tests mock it asjest.fn(), both of which are compatible with the wider union type. The single usage oft()within the function body works correctly with both types in the union.frontend/package.json (1)
197-197: No action required. [email protected] is compatible with react ^17.0.1 and i18next ^23.16.8, has no security vulnerabilities, and the codebase usage relies on stable APIs that are compatible with this version.frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
6-7: LGTM! Test mock aligns with broadened type signature.The simplified mock function type correctly reflects the production code's acceptance of both
TFunctionand plain string-mapping functions.frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx (1)
15-18: LGTM! Type broadening supports flexible translation function usage.The updated signature accepts both
TFunctionfrom i18next and plain string-mapping functions, enabling easier testing while maintaining production compatibility.frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
11-12: LGTM! Custom translation type simplifies interface.The
TranslationFunctiontype provides a cleaner interface while supporting both key-only and key-with-options translation patterns.frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
30-39: LGTM! API enhancement enables fluent usage and type flexibility.The updated signature accepts both
TFunctionand plain string-mapping functions, and returning the extension enables method chaining. This is a backward-compatible improvement.frontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.ts (1)
46-80: LGTM! Tests correctly updated for new API.The test properly validates:
- The broadened translation function type via
mockFnalias- The fluent API by checking return values
- Translation behavior remains unchanged
frontend/public/components/error.tsx (1)
95-122: Fragment wrapping is required for type compatibility with React.FC.The
LoginErrorMessagecomponent is typed asReact.FC, which requires aJSX.Elementreturn type. Sincet()from react-i18next v15.1.4 returns a string, the fragments (<>{t(...)}</>) are necessary to convert the string result to JSX. Removing them would cause TypeScript type errors. This is not unnecessary overhead but a required type safety measure.Likely an incorrect or invalid review comment.
dynamic-demo-plugin/package.json (1)
37-37: Expand verification scope for react-i18next: upgrade path is v11→v23, not v11→v15.The react-i18next upgrade to v15.1.4 is accompanied by i18next v23.16.8, spanning 12 major versions rather than 4. Breaking changes across v11→v23 are more extensive than v11→v15 alone.
Current state:
- Promise-based API (v13+ breaking change) is correctly implemented:
i18n.init({...}).then(...).catch(...)- Modern APIs only: useTranslation hooks and Trans component throughout; no deprecated withTranslation HOC
- useSuspense and fragment-wrapping are configured
Verify:
- TypeScript type augmentation moved to i18next package (v14→v15 change) — confirm in tsconfig/type declarations
- returnNull default behavior (changed to false in later versions) — confirm expected null-return behavior not relied upon
- The "fragment-wrapping pattern" mentioned in the original review—this term does not appear in official migration docs; clarify if this refers to React 18 Suspense boundaries or a different pattern
frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx (1)
57-58: LGTM! Improved fallback translation function.The fallback translation function now returns the key itself instead of an empty string, which is better for debugging and provides a meaningful fallback when the translation function is unavailable.
frontend/packages/console-plugin-sdk/src/utils/useTranslationExt.ts (1)
11-18: This is a new file, not a modification of existing code, so the concern about "removal of explicit TFunction typing" is invalid.The file
useTranslationExt.tswas created as part of this commit (not modified from a prior version). The type design usingUseTranslationOptions<undefined>is intentional and aligns with react-i18next v15.1.4 behavior—when no namespace is explicitly supplied, the library uses the configured default namespace and falls back to undefined for generic inference. All existing usages inuseTranslatedExtensions.tsand tests inuseTranslationExt.spec.tspass without type errors, confirming that the callback type inference works correctly. No compatibility issues exist.Likely an incorrect or invalid review comment.
frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx
Show resolved
Hide resolved
krishagarwal278
force-pushed
the
CONSOLE-4778-new
branch
from
50ae957 to
0b2b36a
Compare
openshift-merge-robot
removed
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
781-781: Consider aligning withTranslationFunctiontype for consistency.The union type
TFunction | ((key: string) => string)differs from theTranslationFunctiontype introduced in other parts of this PR (e.g.,getVSphereHealth.ts). TheTranslationFunctiontype includes an optionaloptionsparameter:(key: string, options?: Record<string, unknown>) => string, which provides better alignment with i18next's actual usage patterns.While the current usage in this file (line 825) doesn't require the options parameter, standardizing on
TranslationFunctionwould improve consistency and future-proof the signature.🔎 Suggested alignment
- t: TFunction | ((key: string) => string), + t: TFunction | TranslationFunction,And add the import at the top:
import { TranslationFunction } from '@console/vsphere-plugin/src/components/getVSphereHealth';Or define it locally if cross-package imports aren't desired.
📜 Review details
Configuration used: Organization 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 ignored due to path filters (2)
dynamic-demo-plugin/yarn.lockis excluded by!**/yarn.lock,!**/*.lockfrontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
dynamic-demo-plugin/package.jsondynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsxfrontend/package.jsonfrontend/packages/console-app/src/components/nodes/NodesPage.tsxfrontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/console-plugin-sdk/src/utils/useTranslationExt.tsfrontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.tsfrontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsxfrontend/packages/dev-console/src/components/import/__tests__/import-submit-utils.spec.tsfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/packages/metal3-plugin/src/validations/validations.tsxfrontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsxfrontend/packages/operator-lifecycle-manager/src/status/csv-status.tsfrontend/packages/topology/src/__tests__/TopologyShortcuts.spec.tsxfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxfrontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsxfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/vsphere-plugin/src/components/utils.tsfrontend/public/components/about-modal.tsxfrontend/public/components/cron-job.tsxfrontend/public/components/custom-resource-definition.tsxfrontend/public/components/error.tsxfrontend/public/components/modals/column-management-modal.tsxfrontend/public/components/modals/configure-count-modal.tsxfrontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsxfrontend/public/components/pod.tsxfrontend/public/module/k8s/get-resources.ts
💤 Files with no reviewable changes (1)
- dynamic-demo-plugin/src/components/ProjectOverview/Inventory.tsx
🚧 Files skipped from review as they are similar to previous changes (22)
- frontend/public/components/monitoring/receiver-forms/alert-manager-receiver-forms.tsx
- frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx
- frontend/packages/console-plugin-sdk/src/utils/useTranslationExt.ts
- dynamic-demo-plugin/package.json
- frontend/public/components/modals/configure-count-modal.tsx
- frontend/public/components/custom-resource-definition.tsx
- frontend/packages/operator-lifecycle-manager/src/status/csv-status.ts
- frontend/public/components/about-modal.tsx
- frontend/packages/vsphere-plugin/src/components/utils.ts
- frontend/public/components/cron-job.tsx
- frontend/packages/metal3-plugin/src/validations/validations.tsx
- frontend/packages/console-plugin-sdk/src/utils/tests/extension-i18n.spec.ts
- frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx
- frontend/public/module/k8s/get-resources.ts
- frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts
- frontend/public/components/modals/column-management-modal.tsx
- frontend/packages/dev-console/src/components/import/tests/import-submit-utils.spec.ts
- frontend/public/components/pod.tsx
- frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx
- frontend/packages/topology/src/tests/TopologyShortcuts.spec.tsx
- frontend/packages/dev-console/src/components/catalog/providers/tests/useConsoleSamples.spec.ts
- frontend/packages/console-app/src/components/nodes/NodesPage.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/packages/vsphere-plugin/src/components/persist.tsfrontend/public/components/error.tsxfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/package.jsonfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx
🧬 Code graph analysis (4)
frontend/packages/vsphere-plugin/src/components/persist.ts (1)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
TranslationFunction(11-11)
frontend/public/components/error.tsx (3)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/public/components/events.jsx (1)
error(386-386)frontend/public/components/namespace.jsx (1)
error(857-857)
frontend/packages/dev-console/src/components/import/import-submit-utils.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)
🔇 Additional comments (4)
frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx (1)
57-58: LGTM! Good defensive fallback pattern.The fallback translation function ensures
getVSphereHealthalways receives a valid translation function, even whentis undefined. The identity function(key: string) => keyis a sensible fallback that returns the untranslated key.frontend/packages/vsphere-plugin/src/components/persist.ts (1)
17-17: LGTM! Consistent adoption ofTranslationFunctiontype.The migration from
TFunction<'vsphere-plugin'>toTranslationFunctionis applied consistently across all function signatures in this file. The usage patterns throughout the file (including interpolation with options) are fully compatible with theTranslationFunctionsignature.Also applies to: 95-95, 132-132, 252-252, 363-363, 554-554, 602-602
frontend/package.json (2)
180-180: i18next upgrade looks properly addressed. The breaking changes from v21 → v23 were comprehensively handled across the codebase (per TranslationFunction type updates and test refactoring), and version 23.16.8 exists with no reported security vulnerabilities. TypeScript 5.7.2 satisfies the v23 requirement for TS v5.
205-205: This upgrade is compatible with React 17.0.1 — no action required.react-i18next 15.1.4 has peer dependency
react >= 16.8.0, and your current React 17.0.1 satisfies this. The i18next dependency (>= 23.2.3) is also met by your pinned version 23.16.8. There are no known security vulnerabilities for v15.1.4, and no breaking changes affecting JavaScript usage with React 17.
frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts
Outdated
Show resolved
Hide resolved
openshift-ci
bot
added
the
kind/i18n
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/public/locales/en/public.json (1)
1779-1787: Improve consistency and clarity of new error messages.The new localization entries could benefit from the following improvements:
Resource name formatting (Lines 1779, 1783): "kubecontrollermanager" should follow Kubernetes naming conventions. Consider using "KubeControllerManager" or "kube-controller-manager" to match the API resource format and improve readability.
Placeholder spacing inconsistency (Line 1781): The placeholder
{{ expectedValues }}has spaces around the variable, while line 1780 uses{{cm}}without spaces. Standardize spacing for consistency—existing patterns in the file predominantly use no spaces (e.g., line 1780).Generic error message (Line 1787): "Unexpected error" provides minimal context. Consider making it more descriptive or specific to the operation that failed to aid user troubleshooting.
Capitalization alignment: Line 1781 uses "configMap" while existing patterns capitalize resource types (e.g., line 523: "ConfigMaps"). Consider "ConfigMap" for consistency.
🔎 Suggested improvements
- "Failed to load kubecontrollermanager": "Failed to load kubecontrollermanager", + "Failed to load kubecontrollermanager": "Failed to load KubeControllerManager", "Failed to parse cloud provider config {{cm}}": "Failed to parse cloud provider config {{cm}}", - "The following content was expected to be defined in the configMap: {{ expectedValues }}": "The following content was expected to be defined in the configMap: {{ expectedValues }}", + "The following content was expected to be defined in the configMap: {{expectedValues}}": "The following content was expected to be defined in the ConfigMap: {{expectedValues}}", "Failed to persist {{secret}}": "Failed to persist {{secret}}", - "Failed to patch kubecontrollermanager": "Failed to patch kubecontrollermanager", + "Failed to patch kubecontrollermanager": "Failed to patch KubeControllerManager", "Failed to patch cloud provider config": "Failed to patch cloud provider config", "Failed to add taint to nodes": "Failed to add taint to nodes", "Failed to patch infrastructure spec": "Failed to patch infrastructure spec", - "Unexpected error": "Unexpected error", + "Unexpected error": "An unexpected error occurred",
📜 Review details
Configuration used: Organization 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 (3)
frontend/packages/operator-lifecycle-manager/locales/en/olm.jsonfrontend/packages/vsphere-plugin/locales/en/vsphere-plugin.jsonfrontend/public/locales/en/public.json
💤 Files with no reviewable changes (2)
- frontend/packages/operator-lifecycle-manager/locales/en/olm.json
- frontend/packages/vsphere-plugin/locales/en/vsphere-plugin.json
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
frontend/public/locales/en/public.json
krishagarwal278
force-pushed
the
CONSOLE-4778-new
branch
from
489994c to
db37e00
Compare
|
/retest |
1 similar comment
|
/retest |
|
/retest ci/prow/e2e-gcp-console |
|
@jhadvig: The The following commands are available to trigger optional jobs: Use 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. |
|
Flake? |
|
/retest |
|
@krishagarwal278: This pull request references CONSOLE-4778 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
krishagarwal278
force-pushed
the
CONSOLE-4778-new
branch
from
44f6f72 to
acc91a4
Compare
|
@krishagarwal278: This pull request references CONSOLE-4778 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts:
- Around line 287-288: Remove the duplicate `ConsoleTFunction` type declaration
in `extension-i18n.ts` and instead import and re-export the single source of
truth `ConsoleTFunction` from the public API `console-dynamic-plugin-sdk` so all
consumers use the same type; specifically, delete the local type alias and add
an import of `ConsoleTFunction` and re-export it (e.g., export {
ConsoleTFunction } from 'console-dynamic-plugin-sdk') so `extension-i18n.ts`
references the shared definition.
🧹 Nitpick comments (2)
frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
30-31: Duplicate ConsoleTFunction definition — re-export from SDK instead.This duplicates the type already defined in
console-dynamic-plugin-sdk/src/extensions/console-types.ts. To maintain a single source of truth:♻️ Suggested refactor
-export type ConsoleTFunction = TFunction | ((key: string, options?: any) => string); +export { ConsoleTFunction } from '@console/dynamic-plugin-sdk';Or if the SDK re-exports it:
+import { ConsoleTFunction } from '@console/dynamic-plugin-sdk'; -export type ConsoleTFunction = TFunction | ((key: string, options?: any) => string); +export { ConsoleTFunction };The
translateExtensionsignature change itself is correct and maintains backward compatibility with existing callers.frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx (1)
67-69: Consider consolidating fallback handling.Line 68 has a secondary inline fallback (
t?.('vsphere-plugin~No errors reported') || 'No errors reported'), butfallbackTon line 58 already guarantees a non-null translation function is passed togetVSphereHealth.However, this specific case is in
healthHandlerwheretcomes from the external caller and may genuinely beundefined. The dual fallback is intentional here — thefallbackTguardsgetVSphereHealth, but the inline fallback guards the directt?.()call.This is fine as-is, though if you want consistency, you could also use
(t || fallbackT)('vsphere-plugin~No errors reported')to eliminate the hardcoded duplicate string.♻️ Optional cleanup
case HealthState.OK: - message = t?.('vsphere-plugin~No errors reported') || 'No errors reported'; + message = (t || fallbackT)('vsphere-plugin~No errors reported'); break;
📜 Review details
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 ignored due to path filters (2)
dynamic-demo-plugin/yarn.lockis excluded by!**/yarn.lock,!**/*.lockfrontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
dynamic-demo-plugin/package.jsonfrontend/package.jsonfrontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.tsfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/console-plugin-sdk/src/utils/useTranslationExt.tsfrontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsxfrontend/packages/dev-console/src/components/import/import-submit-utils.tsfrontend/packages/metal3-plugin/src/validations/validations.tsxfrontend/packages/vsphere-plugin/locales/en/vsphere-plugin.jsonfrontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsxfrontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsxfrontend/packages/vsphere-plugin/src/components/getVSphereHealth.tsfrontend/packages/vsphere-plugin/src/components/persist.tsfrontend/packages/vsphere-plugin/src/components/utils.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/package.json
- frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts
- frontend/packages/console-plugin-sdk/src/utils/useTranslationExt.ts
- frontend/packages/vsphere-plugin/locales/en/vsphere-plugin.json
- frontend/packages/metal3-plugin/src/validations/validations.tsx
- dynamic-demo-plugin/package.json
- frontend/packages/vsphere-plugin/src/components/VSphereOperatorStatuses.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx (3)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
ConsoleTFunction(287-287)frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
ConsoleTFunction(30-30)frontend/packages/vsphere-plugin/src/components/getVSphereHealth.ts (1)
getVSphereHealth(17-91)
frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx (3)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
ConsoleTFunction(287-287)frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
ConsoleTFunction(30-30)
frontend/packages/dev-console/src/components/import/import-submit-utils.ts (3)
frontend/packages/dev-console/src/components/catalog/providers/__tests__/useConsoleSamples.spec.ts (1)
t(6-7)frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
ConsoleTFunction(287-287)frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
ConsoleTFunction(30-30)
frontend/packages/vsphere-plugin/src/components/utils.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
ConsoleTFunction(287-287)frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
ConsoleTFunction(30-30)
frontend/packages/vsphere-plugin/src/components/persist.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
ConsoleTFunction(287-287)frontend/packages/console-plugin-sdk/src/utils/extension-i18n.ts (1)
ConsoleTFunction(30-30)
🔇 Additional comments (15)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
14-14: LGTM on the TFunction import.Clean type import from i18next for the new
ConsoleTFunctionunion type.frontend/packages/dev-console/src/components/catalog/providers/useConsoleSamples.tsx (2)
4-4: Clean import migration to ConsoleTFunction.Correctly imports from the SDK's public API surface. This aligns with the broader i18next upgrade pattern.
14-14: LGTM on signature update.The
normalizeConsoleSamplesfunction now acceptsConsoleTFunction, which is compatible with both thetfromuseTranslation()(line 52) and any custom translation functions plugins might provide.frontend/packages/vsphere-plugin/src/components/ClusterOverview/VSphereStatus.tsx (1)
58-59: Good fallback pattern — identity function preserves debug context.The
fallbackTreturning the key is a meaningful improvement over the previous empty-string fallback. If translations fail to load, users see the key (e.g.,vsphere-plugin~No errors reported) rather than blank text, aiding debugging.frontend/packages/dev-console/src/components/import/import-submit-utils.ts (2)
3-3: LGTM on import consolidation.
ConsoleTFunctioncorrectly imported from the SDK. ThePerspectiveimport on the same line appears to be an existing import, not part of this change.
779-780: Function signature correctly maintains i18next compatibility.The
createOrUpdateResourcesfunction parametert: ConsoleTFunctionaccepts the translation function fromuseTranslation()(react-i18next), which returnsTFunction—a direct member of theConsoleTFunctionunion type. All callers (AddServerlessFunction, ImportForm, ImportSamplePage) pass compatible translation functions obtained from the hook, ensuring backward compatibility with the existing codebase.frontend/packages/vsphere-plugin/src/components/utils.ts (2)
2-2: Import path looks correct.Using
@console/dynamic-plugin-sdkis the appropriate public API surface for consuming SDK types in external plugins. This aligns with the SDK's re-export pattern.
26-37: Type signature change is consistent with the broader i18next upgrade.Switching from
TFunction<'vsphere-plugin'>toConsoleTFunctiontrades namespace-specific key validation for broader compatibility with the updated i18next types. TheConsoleTFunctionunion type (TFunction | ((key: string, options?: any) => string)) is more permissive but necessary for this upgrade.The function logic remains sound—proper type narrowing for
Error,string, andStringinstances, with a fallback translation for unexpected error types.frontend/packages/vsphere-plugin/src/components/persist.ts (7)
2-2: Import addition is appropriate.Type-only import from the SDK's public API surface follows best practices for tree-shaking and module resolution.
94-106: Function signature update looks good.The
getPatchKubeControllerManagerOpsignature change toConsoleTFunctionis consistent with the codebase-wide refactor. The error handling withPersistErrorandgetErrorMessage(t, e)properly propagates localized error messages.
131-145: updateYamlFormat signature updated consistently.The i18n interpolation syntax
{{cm}}on line 142 follows correct i18next conventions for variable substitution in translation keys.
251-312: updateIniFormat updates are consistent with the refactor pattern.The function correctly uses
ConsoleTFunctionand the error message interpolation at lines 304-307 properly uses the{{ }}syntax for bothcmandexpectedValuesvariables.
360-396: getPersistProviderConfigMapOp signature change is appropriate.This function orchestrates between YAML and INI format updates, both of which have been updated to use
ConsoleTFunction. The try-catch fallback pattern (YAML → INI) remains sound.
543-595: runPatches parameter typing updated correctly.The object destructuring pattern with
t: ConsoleTFunctionat line 552 maintains type safety for all internalPersistErrorcalls. The error handling pattern withPromise.allSettledfor taints (lines 581-588) is a good practice for partial failure scenarios.
597-653: Exportedpersistfunction signature updated appropriately.This is the public API entry point for the persistence flow. The change from
TFunction<'vsphere-plugin'>toConsoleTFunctionmaintains backward compatibility sinceTFunctionis a subset of the union type. Callers usinguseTranslation('vsphere-plugin')will continue to work seamlessly.The dry-run pattern (lines 642-643) followed by actual execution is a solid approach for validating K8s operations before committing changes.
| export type ConsoleTFunction = TFunction | ((key: string, options?: any) => string); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all ConsoleTFunction definitions in the codebase
rg -n 'export type ConsoleTFunction' --type=tsRepository: openshift/console
Length of output: 379
🏁 Script executed:
#!/bin/bash
# Examine both files to understand context and imports
echo "=== File 1: console-dynamic-plugin-sdk/src/extensions/console-types.ts ==="
sed -n '280,295p' frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts
echo ""
echo "=== File 2: console-plugin-sdk/src/utils/extension-i18n.ts ==="
sed -n '1,40p' frontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsRepository: openshift/console
Length of output: 2006
🏁 Script executed:
#!/bin/bash
# Check package dependencies to understand if console-plugin-sdk depends on console-dynamic-plugin-sdk
echo "=== console-plugin-sdk package.json ==="
cat frontend/packages/console-plugin-sdk/package.json | jq '.dependencies, .peerDependencies' 2>/dev/null || grep -A 10 '"dependencies"' frontend/packages/console-plugin-sdk/package.json
echo ""
echo "=== Check if ConsoleTFunction is exported from console-dynamic-plugin-sdk's public API ==="
rg -n 'ConsoleTFunction' frontend/packages/console-dynamic-plugin-sdk/src/index.ts
echo ""
echo "=== Check all usages of ConsoleTFunction across codebase ==="
rg -n 'ConsoleTFunction' --type=ts -A 2 | head -50Repository: openshift/console
Length of output: 4472
🏁 Script executed:
#!/bin/bash
# Check console-dynamic-plugin-sdk's index and what it exports
echo "=== console-dynamic-plugin-sdk index file ==="
head -100 frontend/packages/console-dynamic-plugin-sdk/src/index.ts
echo ""
echo "=== Check how ConsoleTFunction is used in console-plugin-sdk ==="
rg -n 'ConsoleTFunction' frontend/packages/console-plugin-sdk --type=ts -B 2 -A 2
echo ""
echo "=== Check if console-plugin-sdk references console-dynamic-plugin-sdk ==="
grep -r '@console/dynamic-plugin-sdk' frontend/packages/console-plugin-sdk/src --include="*.ts" --include="*.tsx"Repository: openshift/console
Length of output: 5741
🏁 Script executed:
#!/bin/bash
# Check what's exported from ./extensions
echo "=== frontend/packages/console-dynamic-plugin-sdk/src/extensions/index.ts ==="
cat frontend/packages/console-dynamic-plugin-sdk/src/extensions/index.ts | head -80
echo ""
echo "=== Check if ConsoleTFunction is in the extensions exports ==="
grep -n 'ConsoleTFunction' frontend/packages/console-dynamic-plugin-sdk/src/extensions/index.tsRepository: openshift/console
Length of output: 1354
🏁 Script executed:
#!/bin/bash
# Verify ConsoleTFunction is actually exported from console-dynamic-plugin-sdk public API
echo "=== Verify ConsoleTFunction is exported from public API via wildcard export ==="
grep -n 'export type ConsoleTFunction' frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts
echo ""
echo "=== Check if extension-i18n.ts in plugin-sdk is imported/used elsewhere ==="
rg -n 'from.*extension-i18n' frontend/packages --type=ts
echo ""
echo "=== Check what is imported from console-plugin-sdk in internal code ==="
rg '@console/plugin-sdk.*extension-i18n' frontend/packages --type=tsRepository: openshift/console
Length of output: 861
🏁 Script executed:
#!/bin/bash
# Check if ConsoleTFunction is exported from console-plugin-sdk's public API
echo "=== Check console-plugin-sdk public API exports ==="
head -50 frontend/packages/console-plugin-sdk/src/index.ts
echo ""
echo "=== Full console-plugin-sdk index.ts ==="
cat frontend/packages/console-plugin-sdk/src/index.tsRepository: openshift/console
Length of output: 362
🏁 Script executed:
#!/bin/bash
# Explore console-plugin-sdk structure
echo "=== Explore console-plugin-sdk directory ==="
find frontend/packages/console-plugin-sdk -type f -name "*.ts" | head -30
echo ""
echo "=== Check package.json of console-plugin-sdk ==="
cat frontend/packages/console-plugin-sdk/package.json | jq '.main, .exports, .files' 2>/dev/null || cat frontend/packages/console-plugin-sdk/package.json | grep -E '"main"|"exports"|"files"'Repository: openshift/console
Length of output: 1401
🏁 Script executed:
#!/bin/bash
# Find and examine the actual index file
echo "=== List all files in console-plugin-sdk/src ==="
ls -la frontend/packages/console-plugin-sdk/src/ 2>/dev/null || find frontend/packages/console-plugin-sdk -maxdepth 2 -type f
echo ""
echo "=== Check if index.ts exists anywhere in console-plugin-sdk ==="
find frontend/packages/console-plugin-sdk -name 'index.ts' -o -name 'index.js'
echo ""
echo "=== Check console-plugin-sdk structure more carefully ==="
tree -L 3 frontend/packages/console-plugin-sdk/ 2>/dev/null || find frontend/packages/console-plugin-sdk/src -type d | head -20Repository: openshift/console
Length of output: 1403
🏁 Script executed:
#!/bin/bash
# Check package.json main field and what's actually used
echo "=== console-plugin-sdk package.json details ==="
cat frontend/packages/console-plugin-sdk/package.json
echo ""
echo "=== Search for imports of ConsoleTFunction from console-plugin-sdk ==="
rg 'from.*["\']@console/plugin-sdk' frontend/packages --type=ts | grep -i 'consoletfunction' || echo "No imports of ConsoleTFunction from @console/plugin-sdk found"
echo ""
echo "=== Verify all imports of ConsoleTFunction come from console-dynamic-plugin-sdk ==="
rg "import.*ConsoleTFunction" frontend/packages --type=ts | grep -v console-plugin-sdkRepository: openshift/console
Length of output: 667
🏁 Script executed:
#!/bin/bash
# Check imports of ConsoleTFunction from console-plugin-sdk
echo "=== Search for imports of ConsoleTFunction from console-plugin-sdk ==="
rg 'from.*@console/plugin-sdk' frontend/packages --type=ts | grep -i 'consoletfunction' || echo "No direct imports of ConsoleTFunction from @console/plugin-sdk found"
echo ""
echo "=== Verify all ConsoleTFunction imports come from console-dynamic-plugin-sdk ==="
rg "import.*ConsoleTFunction" frontend/packages --type=ts
echo ""
echo "=== Check if anyone imports from console-plugin-sdk/extension-i18n ==="
rg "extension-i18n" frontend/packages --type=ts | grep -v "console-plugin-sdk/src"Repository: openshift/console
Length of output: 1199
Consolidate ConsoleTFunction type to single source of truth.
Duplicate type definition detected in console-plugin-sdk/src/utils/extension-i18n.ts (line 30). Since console-dynamic-plugin-sdk is already the public API surface and all consumers import ConsoleTFunction from there, consolidate by having extension-i18n.ts import and re-export from console-dynamic-plugin-sdk instead of defining locally. The type correctly bridges i18next's strict TFunction with looser plugin-provided translation functions.
🤖 Prompt for AI Agents
In @frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts
around lines 287 - 288, Remove the duplicate `ConsoleTFunction` type declaration
in `extension-i18n.ts` and instead import and re-export the single source of
truth `ConsoleTFunction` from the public API `console-dynamic-plugin-sdk` so all
consumers use the same type; specifically, delete the local type alias and add
an import of `ConsoleTFunction` and re-export it (e.g., export {
ConsoleTFunction } from 'console-dynamic-plugin-sdk') so `extension-i18n.ts`
references the shared definition.
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/verified by yanpzhan |
openshift-ci-robot
added
the
verified
|
@yanpzhan: 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. |
|
/retest |
openshift-ci-robot
removed
the
verified
|
@krishagarwal278: This pull request references CONSOLE-4778 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
|
/lgtm |
|
@krishagarwal278: This pull request references CONSOLE-4778 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/public/locales/en/public.json:
- Around line 1792-1794: The three i18n entries "Failed to parse cloud provider
config {{cm}}", "The following content was expected to be defined in the
configMap: {{ expectedValues }}", and "Failed to persist {{secret}}" are
inconsistent; standardize placeholder formatting to the spaced style (e.g.,
change {{cm}} -> {{ cm }} and {{secret}} -> {{ secret }}) and fix resource
casing by changing "configMap" to "ConfigMap" so the second key becomes "The
following content was expected to be defined in the ConfigMap: {{ expectedValues
}}".
🧹 Nitpick comments (1)
frontend/public/locales/en/public.json (1)
1791-1791: Consider hyphenated form for "kubecontrollermanager" readability.The Kubernetes component is typically written as "kube-controller-manager" in documentation and CLI references. While the CamelCase
KubeControllerManagermatches the API resource kind, user-facing error messages would benefit from the hyphenated form for clarity.Optional: Improve readability
- "Failed to load kubecontrollermanager": "Failed to load kubecontrollermanager", + "Failed to load kubecontrollermanager": "Failed to load kube-controller-manager", ... - "Failed to patch kubecontrollermanager": "Failed to patch kubecontrollermanager", + "Failed to patch kubecontrollermanager": "Failed to patch kube-controller-manager",Note: The keys remain unchanged to match code references; only the displayed values are updated for user readability.
Also applies to: 1795-1795
📜 Review details
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 (1)
frontend/public/locales/en/public.json
🔇 Additional comments (1)
frontend/public/locales/en/public.json (1)
1790-1799: New error message locale entries look good overall.The additions provide necessary i18n support for cloud provider configuration, node tainting, and infrastructure patching error scenarios. The keys follow the existing pattern of identity mapping (key equals value for English locale), and the JSON structure is valid.
| "Failed to parse cloud provider config {{cm}}": "Failed to parse cloud provider config {{cm}}", | ||
| "The following content was expected to be defined in the configMap: {{ expectedValues }}": "The following content was expected to be defined in the configMap: {{ expectedValues }}", | ||
| "Failed to persist {{secret}}": "Failed to persist {{secret}}", |
There was a problem hiding this comment.
Inconsistent placeholder formatting and resource casing.
A couple of i18n consistency nits:
-
Placeholder spacing:
{{cm}}and{{secret}}use no spaces, while{{ expectedValues }}uses spaces. Pick one style—existing entries in this file predominantly use spaces (e.g.,{{ fileName }},{{ namespace }}). -
Resource casing: Line 1793 uses lowercase "configMap" but Kubernetes resource names should be PascalCase (
ConfigMap) to match official terminology and other entries in this file (see lines 292, 308, 1547, 1612).
Suggested fix
- "Failed to parse cloud provider config {{cm}}": "Failed to parse cloud provider config {{cm}}",
- "The following content was expected to be defined in the configMap: {{ expectedValues }}": "The following content was expected to be defined in the configMap: {{ expectedValues }}",
- "Failed to persist {{secret}}": "Failed to persist {{secret}}",
+ "Failed to parse cloud provider config {{ cm }}": "Failed to parse cloud provider config {{ cm }}",
+ "The following content was expected to be defined in the ConfigMap: {{ expectedValues }}": "The following content was expected to be defined in the ConfigMap: {{ expectedValues }}",
+ "Failed to persist {{ secret }}": "Failed to persist {{ secret }}",📝 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.
| "Failed to parse cloud provider config {{cm}}": "Failed to parse cloud provider config {{cm}}", | |
| "The following content was expected to be defined in the configMap: {{ expectedValues }}": "The following content was expected to be defined in the configMap: {{ expectedValues }}", | |
| "Failed to persist {{secret}}": "Failed to persist {{secret}}", | |
| "Failed to parse cloud provider config {{ cm }}": "Failed to parse cloud provider config {{ cm }}", | |
| "The following content was expected to be defined in the ConfigMap: {{ expectedValues }}": "The following content was expected to be defined in the ConfigMap: {{ expectedValues }}", | |
| "Failed to persist {{ secret }}": "Failed to persist {{ secret }}", |
🤖 Prompt for AI Agents
In @frontend/public/locales/en/public.json around lines 1792 - 1794, The three
i18n entries "Failed to parse cloud provider config {{cm}}", "The following
content was expected to be defined in the configMap: {{ expectedValues }}", and
"Failed to persist {{secret}}" are inconsistent; standardize placeholder
formatting to the spaced style (e.g., change {{cm}} -> {{ cm }} and {{secret}}
-> {{ secret }}) and fix resource casing by changing "configMap" to "ConfigMap"
so the second key becomes "The following content was expected to be defined in
the ConfigMap: {{ expectedValues }}".
|
/label tide/merge-method-squash |
openshift-ci
bot
added
tide/merge-method-squash
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cajieh, krishagarwal278, 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 |
|
@krishagarwal278: The following tests 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. |
9 participants
Summary by CodeRabbit
Chores
Refactor
Localization
✏️ Tip: You can customize this high-level summary in your review settings.