Skip to content

Conversation

@empovit
Copy link
Member

@empovit empovit commented Jan 8, 2026

  • Update Console SDK 0.0.11 → 1.4.0
  • Upgrade PatternFly v4 → v6
  • Upgrade to Node.js 20
  • Enable TypeScript strict mode
  • Fix webpack security vulnerability
  • Follow template CSS naming conventions (ng- prefix)
  • Bump Helm chart version to 0.3.0 (backward compatible)

Summary by CodeRabbit

  • New Features

    • Added comprehensive internationalization (i18n) support with automated extraction and defaults.
    • Redesigned GPU selector component for enhanced user experience.
  • Documentation

    • Updated system requirements: OpenShift 4.19+, Node.js 20+.
  • Style

    • Migrated styling to PatternFly v6 standards and conventions.
  • Chores

    • Modernized development tooling, build infrastructure, and linting configurations.

✏️ Tip: You can customize this high-level summary in your review settings.

- Update Console SDK 0.0.11 → 1.4.0
- Upgrade PatternFly v4 → v6
- Upgrade to Node.js 20
- Enable TypeScript strict mode
- Fix webpack security vulnerability
- Follow template CSS naming conventions (ng- prefix)
- Bump Helm chart version to 0.3.0 (backward compatible)
@openshift-ci
Copy link

openshift-ci bot commented Jan 8, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: empovit

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Jan 8, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026

Walkthrough

This PR modernizes the project's tooling and infrastructure. It updates ESLint, Prettier, and Stylelint configurations to YAML format with enhanced rule sets. The Dockerfile upgrades base images from UBI8/Node16 to UBI9/Node20, and package.json dependencies are bumped significantly. New i18n scripts and a CustomJSONLexer are added for translation automation. Multiple component files update imports to use local shared modules and apply PatternFly v6 styling classes. TypeScript and webpack configurations are updated for stricter type checking and production-ready builds.

Changes

Cohort / File(s) Summary
Configuration Files - ESLint & Linting
.eslintrc, .eslintrc.yml, i18n-scripts/lexers.js
Migrated ESLint config from JSON to YAML format (.eslintrc removed, .eslintrc.yml added) with TypeScript and React linting rules. Added custom JSON lexer for i18next integration.
Configuration Files - Styling & Formatting
.prettierrc.yml, .stylelintrc.yaml, .gitignore
Updated Prettier config (removed tabWidth, semi, jsxSingleQuote, proseWrap keys). Added new Stylelint configuration enforcing PatternFly conventions, dark-mode friendliness, and CSS best practices. Extended .gitignore for parcel cache.
Build & Deployment
Dockerfile, webpack.config.ts, package.json
Upgraded Dockerfile base images to UBI9/Node20 with updated build paths. Enhanced webpack config with production mode optimization, hash-based filenames, SCSS support, and dev-server CORS headers. Major package.json overhaul: dependency bumps, new scripts (type-check, updated build/lint), i18n pipeline scripts, and peerDependencies for i18next.
i18n Infrastructure
i18n-scripts/build-i18n.sh, i18n-scripts/common.js, i18n-scripts/set-english-defaults.js, i18next-parser.config.js
New i18n build script and utilities: common.js provides file I/O helpers, set-english-defaults.js processes locale JSON files with pluralization rules, and i18next-parser.config.js adds custom lexers and sorting.
TypeScript & Compilation
tsconfig.json, src/utils/units.ts
Enabled source maps and strict mode in TypeScript; added ts-node configuration. Updated units.ts public API to accept string | number inputs with internal numeric coercion for broader input tolerance.
Shared Components
src/components/shared/OverviewDetailItem.tsx, src/components/shared/OverviewDetailItem.css
New shared component with CSS module: renders description list items with loading/error states; replaces external import from @openshift-console/plugin-shared.
Component Updates - Import Paths
src/components/ClusterOverview/GPUProviders.tsx, src/components/GPUDashboard/Cards/InfoCard.tsx
Updated OverviewDetailItem imports to use local shared module.
Component Updates - Styling & Layout
src/components/ClusterOverview/GPUStatus.tsx, src/components/GPUDashboard/Cards/MetricsCard.tsx, src/components/NotAvailable.tsx, src/components/GPUDashboard/Cards/WorkloadsCard.css, src/components/GPUDashboard/Cards/WorkloadsCard.tsx
Updated PatternFly utility classes to v6 variants (pf-u-* → pf-v6-u-*). Replaced CSS variables and color imports; updated WorkloadsCard column classes from co-break-word to ng-break-word.
Component Updates - Complex
src/components/GPUDashboard/GPUSelector.tsx, src/components/GPUDashboard/GPUDashboard.tsx, src/components/PodStatus/PodStatus.tsx
Refactored GPUSelector to use SelectList with MenuToggle for improved control. Removed Page wrapper from GPUDashboard. Fixed PodStatus URL metadata references and replaced PatternFly Text components with Content fragments.
Documentation & Versioning
README.md, deployment/console-plugin-nvidia-gpu/Chart.yaml
Updated OpenShift compatibility from 4.12-4.18 to 4.19+ main branch. Bumped chart version from 0.2.5 to 0.3.0.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Suggested labels

approved, lgtm

Suggested reviewers

  • mresvanis
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title describes a major version upgrade effort for OpenShift 4.19+ compatibility, which aligns with the changeset's comprehensive updates across SDK, PatternFly, Node.js, TypeScript, and tooling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@empovit empovit changed the title Upgrade plugin for OpenShift 4.19+ compatibility [WIP] Upgrade plugin for OpenShift 4.19+ compatibility Jan 8, 2026
Copy link

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/GPUDashboard/GPUSelector.tsx (1)

24-31: Prevent selection of loading and error states.

The loading and error state options can still be selected by users, which would set invalid values (like "Loading GPUs" or "No GPU found") as the selected GPU UUID. These options should be explicitly disabled to prevent selection.

🐛 Proposed fix to disable loading/error options
  if (!gpusLoaded) {
    items = [
-      <SelectOption key="disabled" value={t('Loading GPUs')}>
+      <SelectOption key="disabled" value={t('Loading GPUs')} isDisabled>
        <Spinner size="md" /> {t('Loading GPUs')}
      </SelectOption>,
    ];
  } else if (!gpus?.length || gpusError) {
-    items = [<SelectOption key="disabled" value={t('No GPU found')} />];
+    items = [<SelectOption key="disabled" value={t('No GPU found')} isDisabled />];
  } else {
🧹 Nitpick comments (10)
src/utils/units.ts (1)

209-217: Cache the numeric conversion to avoid repeated Number(v) calls.

The function calls Number(v) three times. For consistency with humanizeRatio and humanizePercentage (which use numValue), cache the conversion result.

♻️ Suggested refactor
 export const humanizeCpuCores: HumanizeFunc = (v) => {
-  const value = Number(v) < 1 ? round(Number(v) * 1000) : Number(v);
-  const unit = Number(v) < 1 ? 'm' : '';
+  const numValue = Number(v);
+  const value = numValue < 1 ? round(numValue * 1000) : numValue;
+  const unit = numValue < 1 ? 'm' : '';
   return {
     string: `${formatValue(value)}${unit}`,
     unit,
     value,
   };
 };
src/components/PodStatus/PodStatus.tsx (1)

116-133: Consider consistent paragraph styling.

The informational text uses <Content component="p"> (lines 117-124) while the links section uses plain <p> (lines 125-133). For consistency, consider applying the same approach throughout, unless the distinction is intentional to differentiate content types.

♻️ Proposed refactor for consistency
-          <p>
+          <Content component="p">
             <Link to={`/k8s/ns/${pod.metadata.namespace}/pods/${pod.metadata.name}/logs`}>
               {t('View logs')}
             </Link>
             &emsp;
             <Link to={`/k8s/ns/${pod.metadata.namespace}/pods/${pod.metadata.name}/events`}>
               {t('View events')}
             </Link>
-          </p>
+          </Content>
src/components/shared/OverviewDetailItem.tsx (1)

33-33: Consider avoiding trailing space in className.

The current template literal will produce a trailing space when valueClassName is undefined or empty. While harmless, it's slightly cleaner to avoid it.

♻️ Optional refactor to remove trailing space
-      <dd className={`ng-overview-detail-item__value ${valueClassName || ''}`}>{value}</dd>
+      <dd className={valueClassName ? `ng-overview-detail-item__value ${valueClassName}` : 'ng-overview-detail-item__value'}>{value}</dd>

Alternative using array approach:

-      <dd className={`ng-overview-detail-item__value ${valueClassName || ''}`}>{value}</dd>
+      <dd className={['ng-overview-detail-item__value', valueClassName].filter(Boolean).join(' ')}>{value}</dd>
src/components/GPUDashboard/GPUSelector.tsx (1)

33-35: Consider displaying GPU name instead of UUID.

Line 54 displays selectedGPU?.uuid in the MenuToggle, which shows a UUID to users. Consider displaying a more user-friendly GPU name or model if available in the GPU object.

♻️ Optional enhancement for better UX

If the gpu object has a name or modelName property, consider:

  } else {
    items = gpus.map((gpu) => (
-      <SelectOption key={gpu.uuid} value={gpu.uuid} description={`Model: ${gpu.modelName}`} />
+      <SelectOption key={gpu.uuid} value={gpu.uuid}>
+        {gpu.modelName || gpu.uuid}
+      </SelectOption>
    ));
  }

And update the toggle display:

        >
-          {selectedGPU?.uuid || t('Select GPU')}
+          {selectedGPU?.modelName || selectedGPU?.uuid || t('Select GPU')}
        </MenuToggle>
i18n-scripts/lexers.js (1)

1-30: CustomJSONLexer implementation looks correct.

The lexer correctly:

  • Uses comment-json to parse JSON with comments support
  • Employs a reviver function to extract localization keys matching the %KEY% pattern
  • Handles errors gracefully by logging and returning an empty array
  • Returns keys in the expected { key: ... } format for i18next-parser

The class extends EventEmitter but doesn't emit events. This is likely for interface compatibility with i18next-parser lexer expectations, though it could be removed if not required by the framework.

♻️ Optional: Remove EventEmitter if not required by framework

If i18next-parser doesn't require lexers to extend EventEmitter, you can simplify:

-const EventEmitter = require('events');
 const jsonc = require('comment-json');

 /**
  * Custom JSON parser for localizing keys matching format: /%.+%/
  */
-module.exports.CustomJSONLexer = class extends EventEmitter {
+module.exports.CustomJSONLexer = class {
   extract(content, filename) {
i18next-parser.config.js (1)

1-4: Consider configuring ESLint environment instead of inline disables.

The multiple eslint-disable comments work but suggest ESLint isn't configured for Node.js CommonJS files. Consider adding a .eslintrc override for .js files or setting env: { node: true } to properly recognize require and module.

♻️ Alternative: Configure ESLint for Node.js environment

In your ESLint configuration, add an override for .js files:

{
  "overrides": [
    {
      "files": ["*.js"],
      "env": {
        "node": true
      },
      "rules": {
        "@typescript-eslint/no-var-requires": "off"
      }
    }
  ]
}

This would eliminate the need for inline disable comments.

webpack.config.ts (1)

8-8: Mixed import styles: consider standardizing.

The file uses ES6 imports throughout except for CopyWebpackPlugin. If possible, standardize to ES6 imports for consistency.

♻️ Standardize to ES6 import
-const CopyWebpackPlugin = require('copy-webpack-plugin');
+import CopyWebpackPlugin from 'copy-webpack-plugin';

Note: Only apply if compatible with your TypeScript configuration and plugin version.

i18n-scripts/common.js (2)

14-26: Consider propagating errors to callers.

Errors are logged but the Promise doesn't reject, preventing callers from detecting failures. For build scripts, propagating errors allows proper failure handling.

♻️ Propagate errors to enable proper failure handling
 parseFolder(directory, argFunction, packageDir) {
   return (async () => {
     try {
       const files = await fs.promises.readdir(directory);
       for (const file of files) {
         const filePath = path.join(directory, file);
         await argFunction(filePath, packageDir);
       }
     } catch (e) {
       console.error(`Failed to parseFolder ${directory}:`, e);
+      throw e;
     }
   })();
 },

This allows callers to handle failures appropriately, such as exiting with a non-zero status in build scripts.


27-33: Consider propagating errors for consistency.

Like parseFolder, errors are logged but not thrown. For consistency and to allow callers to handle failures, consider propagating errors.

♻️ Optional: propagate errors
 deleteFile(filePath) {
   try {
     fs.unlinkSync(filePath);
   } catch (e) {
     console.error(`Failed to delete file ${filePath}:`, e);
+    throw e;
   }
 },

Note: Only apply if file deletion failures should halt the build process.

i18n-scripts/set-english-defaults.js (1)

74-76: Consider adding error exit for build failures.

If locale processing fails, the script continues without exiting with a non-zero status. Build scripts should typically fail explicitly to prevent incomplete builds.

♻️ Exit with error status on failure
 (async () => {
-  await common.parseFolder(publicDir, processLocalesFolder);
+  try {
+    await common.parseFolder(publicDir, processLocalesFolder);
+  } catch (error) {
+    console.error('Failed to set English defaults:', error);
+    process.exit(1);
+  }
 })();

This requires updating common.js to propagate errors (see previous comment on that file).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d74eb74 and bd88e0f.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (29)
  • .eslintrc
  • .eslintrc.yml
  • .gitignore
  • .prettierrc.yml
  • .stylelintrc.yaml
  • Dockerfile
  • README.md
  • deployment/console-plugin-nvidia-gpu/Chart.yaml
  • i18n-scripts/build-i18n.sh
  • i18n-scripts/common.js
  • i18n-scripts/lexers.js
  • i18n-scripts/set-english-defaults.js
  • i18next-parser.config.js
  • package.json
  • src/components/ClusterOverview/GPUProviders.tsx
  • src/components/ClusterOverview/GPUStatus.tsx
  • src/components/GPUDashboard/Cards/InfoCard.tsx
  • src/components/GPUDashboard/Cards/MetricsCard.tsx
  • src/components/GPUDashboard/Cards/WorkloadsCard.css
  • src/components/GPUDashboard/Cards/WorkloadsCard.tsx
  • src/components/GPUDashboard/GPUDashboard.tsx
  • src/components/GPUDashboard/GPUSelector.tsx
  • src/components/NotAvailable.tsx
  • src/components/PodStatus/PodStatus.tsx
  • src/components/shared/OverviewDetailItem.css
  • src/components/shared/OverviewDetailItem.tsx
  • src/utils/units.ts
  • tsconfig.json
  • webpack.config.ts
💤 Files with no reviewable changes (1)
  • .eslintrc
🧰 Additional context used
🧬 Code graph analysis (3)
i18n-scripts/lexers.js (2)
i18next-parser.config.js (1)
  • require (2-2)
i18n-scripts/set-english-defaults.js (1)
  • keys (34-34)
src/components/GPUDashboard/GPUDashboard.tsx (1)
src/components/GPUDashboard/Cards/GPUDashboardGraphs.tsx (1)
  • GPUDashboardGraphs (48-146)
i18n-scripts/set-english-defaults.js (3)
i18n-scripts/common.js (2)
  • fs (1-1)
  • path (2-2)
i18next-parser.config.js (1)
  • require (2-2)
i18n-scripts/lexers.js (1)
  • keys (9-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: lint-test
🔇 Additional comments (46)
src/components/NotAvailable.tsx (1)

8-8: LGTM! Correct PatternFly v6 class migration.

The CSS class update from pf-u-color-200 to pf-v6-u-color-200 correctly follows PatternFly v6 naming conventions. The component functionality remains unchanged.

src/components/GPUDashboard/Cards/MetricsCard.tsx (1)

95-95: LGTM! PatternFly v6 utility class update is correct.

The class name update from pf-u-text-align-center to pf-v6-u-text-align-center is consistent with the PatternFly v4 → v6 upgrade described in the PR objectives.

src/utils/units.ts (4)

179-187: LGTM!

The type broadening to accept string | number aligns with the SDK's Humanize type and accommodates string values commonly returned from Prometheus queries.


189-208: LGTM!

Consistent coercion pattern applied across all helpers. The internal humanize function already handles non-finite values, so invalid string inputs gracefully fall back to 0.


219-232: LGTM!

Clean implementation with proper handling of non-finite values and -0. The numValue caching pattern is consistent and efficient.


234-245: LGTM!

Consistent implementation pattern with humanizeRatio. Proper handling of edge cases.

src/components/PodStatus/PodStatus.tsx (2)

3-3: LGTM! PatternFly v6 migration applied correctly.

The addition of Content component and removal of deprecated Text components aligns with the PatternFly v6 upgrade mentioned in the PR objectives.


126-126: Excellent catch! Critical typo fixed.

The correction from pod.metadata.nam to pod.metadata.name fixes broken URLs that would have prevented navigation to pod logs, events, and debug container pages.

Also applies to: 130-130, 140-140

src/components/GPUDashboard/Cards/InfoCard.tsx (1)

3-3: LGTM! Import path updated correctly.

The import has been successfully migrated from the external package to the local shared module. The usage throughout the file is consistent with the new component's API.

src/components/ClusterOverview/GPUProviders.tsx (1)

2-2: LGTM! Import path correctly updated.

The migration from the external package to the local shared module is consistent and correct.

src/components/GPUDashboard/GPUDashboard.tsx (1)

26-55: LGTM! Layout simplified appropriately.

The removal of the Page wrapper and nested PageSection simplifies the component structure while preserving all functionality. The internal layout hierarchy and component composition remain intact.

src/components/shared/OverviewDetailItem.css (1)

1-18: LGTM! CSS follows PatternFly v6 conventions.

The stylesheet correctly:

  • Uses PatternFly v6 design tokens for consistency
  • Follows the ng- prefix naming convention as specified in PR objectives
  • Implements proper text wrapping (lines 16-17) to handle long content gracefully
  • Provides appropriate semantic styling for the dl/dt/dd structure
src/components/shared/OverviewDetailItem.tsx (1)

1-36: LGTM! Well-structured component with good semantic HTML.

The component implementation is solid:

  • Props interface is clean and matches all usage patterns
  • Semantic dl/dt/dd HTML is appropriate for key-value pairs
  • Loading/error states are properly handled with PatternFly v6 components
  • CSS module integration follows best practices
src/components/GPUDashboard/Cards/WorkloadsCard.css (1)

4-9: LGTM! CSS naming convention correctly applied.

The changes properly implement the ng- prefix naming convention mentioned in the PR objectives. The .ng-break-word class with word-break and overflow-wrap properties is appropriate for handling long text content in table cells.

src/components/GPUDashboard/Cards/WorkloadsCard.tsx (2)

49-54: LGTM! CSS class names consistently updated.

All table column class names have been consistently updated from co-break-word to ng-break-word, correctly following the ng- prefix naming convention and matching the CSS file changes.


42-42: PatternFly v6 EmptyState icon prop usage is correct.

The icon={CubesIcon} prop is the correct PatternFly v6 API. The EmptyStateIcon component is no longer directly imported—it's now internal to EmptyState and automatically wraps the icon component you provide.

src/components/ClusterOverview/GPUStatus.tsx (2)

115-115: LGTM! Icon color migration looks correct.

The updated usage of okColor directly (removing .value) aligns with the PatternFly v4→v6 migration. PatternFly v6 icons should properly handle CSS variable strings in the color prop.

Also applies to: 150-150


18-19: The CSS variable and fallback color are correct. The PatternFly v6 token --pf-t--global--color--status--success--default and the fallback color #3D7317 match PatternFly v6 documentation. No centralized token constants file exists in the codebase, so the hardcoded approach is consistent with project conventions.

src/components/GPUDashboard/GPUSelector.tsx (4)

22-22: Good type refinement for strict mode.

The more specific React.ReactElement<SelectOptionProps>[] typing aligns well with TypeScript strict mode and provides better type safety.


47-56: MenuToggle implementation looks correct.

The custom toggle implementation with MenuToggle properly manages refs, click handlers, and accessibility attributes (isExpanded, isDisabled).


2-10: The PatternFly v6 Select imports and implementation are correct. All imported components (SelectOption, SelectList, SelectOptionProps, Select, MenuToggle, MenuToggleElement) are valid exports from @patternfly/react-core and are used properly in the composable Select pattern with correct type safety and handlers.


42-45: The type assertion is safe in this context.

The as string assertion on line 43 is not actually unsafe. All SelectOption values used in this component are strings (both the translated strings on lines 26 and 31, and the GPU UUIDs on line 34). More importantly, the Select component is disabled (via isDisabled on line 20) when the loading and error options are shown, so the onSelect handler is never invoked with those translated strings—only with actual GPU UUID strings. PatternFly v6's onSelect signature accepts string | number, and in this case, only strings are used. The type assertion is redundant but not a safety concern.

Likely an incorrect or invalid review comment.

.gitignore (1)

77-77: Pattern addition looks good.

The .cache_* addition extends parcel-bundler cache ignores to cover dot-variant naming patterns, which aligns with the updated build environment.

.prettierrc.yml (1)

5-6: Verify Prettier config simplification doesn't change formatting behavior.

The removed options (tabWidth, semi, jsxSingleQuote, proseWrap) may have defaults in Prettier that differ from the previous explicit values. Confirm that relying on Prettier's defaults for these options won't unexpectedly change formatting behavior across the codebase.

.eslintrc.yml (1)

1-26: ESLint config is well-structured for TypeScript + React.

The configuration appropriately layers ESLint recommendations, React and TypeScript plugins, and Prettier integration. Disabling react/prop-types in favor of TypeScript's type checking and using auto-detect for React version are modern best practices.

One note: ecmaVersion: latest (line 13) will automatically pick up new ECMAScript versions as ESLint/parser support evolves. For strict reproducibility, consider pinning to a specific version (e.g., 2024). However, given this PR's modernization scope, using latest is likely intentional.

i18n-scripts/build-i18n.sh (1)

1-7: LGTM! Script follows shell best practices.

The script correctly uses strict error handling (set -exuo pipefail) and properly escapes the i18next template variables (\$LOCALE, \$NAMESPACE) to prevent shell expansion. The file pattern and command invocation are appropriate for the i18n extraction workflow.

Dockerfile (2)

1-1: Upgrade to UBI9 and Node.js 20 aligns with PR objectives.

The base image upgrade from ubi8/nodejs-16 to ubi9/nodejs-20 correctly implements the Node.js 20 upgrade mentioned in the PR objectives.


5-11: Build paths correctly updated for new working directory.

The change from root directory to /usr/src/app as the working directory is a good practice, and the artifact copy path in line 11 (--from=build /usr/src/app/dist) correctly reflects this new structure.

tsconfig.json (2)

7-12: TypeScript strict mode enabled as per PR objectives.

The addition of strict: true (line 10) correctly implements the PR objective to enable TypeScript strict mode. The sourceMap: true (line 7) aids debugging, and skipLibCheck: true (line 12) is a common practice to improve compilation performance.


16-22: ts-node configuration supports new i18n tooling.

The ts-node block enables in-project TypeScript execution with CommonJS module resolution, which aligns with the new i18n-scripts infrastructure introduced in this PR. The transpileOnly: true setting is appropriate for build scripts where faster compilation is prioritized.

i18next-parser.config.js (2)

6-6: Good addition: Catalog sorting improves consistency.

Enabling sort: true ensures translation keys are consistently ordered in the generated catalog files, making diffs cleaner and improving maintainability.


15-30: Mixed lexer reference pattern is correct and follows i18next-parser convention.

The lexers configuration correctly uses string references for built-in lexers ('HandlebarsLexer', 'HTMLLexer', 'JavascriptLexer') and a direct class reference for the custom lexer (CustomJSONLexer). This is the documented and intended pattern for i18next-parser: pass string names for built-in lexers and pass the class/function directly when using custom lexers.

.stylelintrc.yaml (1)

1-27: LGTM! Well-structured Stylelint configuration for PatternFly v6.

The configuration appropriately enforces dark mode compatibility and plugin CSS isolation. The rules correctly prevent:

  • Direct color values (enforcing PF CSS variables)
  • Reserved prefixes (.pf-, .co-)
  • Naked element selectors that could impact console layout

The comments clearly explain the rationale for each rule.

webpack.config.ts (5)

43-44: LGTM! SCSS loader chain correctly configured.

The loader chain (sass-loader → css-loader → style-loader) follows webpack best practices for SCSS processing.


53-58: Good addition for module compatibility.

Setting fullySpecified: false improves compatibility with modules that don't use full file extensions in imports, which is a common webpack 5 migration pattern.


64-69: LGTM! DevServer configuration appropriate for development.

The permissive allowedHosts: 'all' and CORS headers are acceptable since they only apply to the development server, not production builds.


76-78: LGTM! CopyWebpackPlugin correctly configured for locale files.

The plugin will copy locale files from the source locales directory to dist/locales, which aligns with the i18n infrastructure added in this PR.


80-84: LGTM! Production optimization settings properly configured.

The configuration appropriately toggles between development (source maps, named chunks) and production (no source maps, deterministic chunks, minification) modes.

package.json (4)

5-8: LGTM! Node.js 20 requirement aligns with modern tooling.

The Node.js 20 engine requirement is appropriate for the updated dependencies and aligns with the Dockerfile upgrade mentioned in the PR objectives.


9-20: LGTM! Script updates align with modernized tooling.

The updated scripts properly support:

  • Production builds with explicit NODE_ENV
  • TypeScript configuration execution via ts-node
  • Integrated i18n workflow
  • CSS linting with stylelint

84-86: LGTM! Correct peerDependency declaration for plugin architecture.

Declaring i18next as a peerDependency is appropriate for a console plugin, ensuring the host console provides a single i18next instance.


22-63: No action needed. PatternFly v6 (including 6.2.2) officially supports React 17 and follows a "two most recent React versions" policy. OpenShift Console SDK 1.4.0 is designed for React 17.x, making this the intended combination. The dependency versions are compatible.

i18n-scripts/common.js (1)

5-13: LGTM! Synchronous operation acceptable for build scripts.

Using lstatSync is appropriate for build-time scripts where blocking is acceptable. The error handling correctly returns false for non-existent paths.

i18n-scripts/set-english-defaults.js (3)

8-19: LGTM! Rule determination follows i18next conventions.

The priority order (WithCount_plural → WithCount → _plural → default) correctly matches i18next naming patterns. The use of includes() is appropriate given standard i18next key conventions.


21-66: LGTM! Transformation logic correctly implements i18next conventions.

The function properly:

  • Validates file structure
  • Preserves existing translations (only fills empty strings)
  • Applies appropriate transformations based on key naming patterns
  • Uses the pluralize library for English pluralization

The file overwriting behavior is acceptable for a build script.


68-72: LGTM! Correctly targets only English locale for defaults.

Filtering by basename === 'en' ensures the script only modifies English locale files, leaving other locales for human translators.

appVersion: latest
description: |
Red Hat OpenShift dynamic console plugin that leverages the NVIDIA GPU operator metrics and serves the respective console-extensions. Requires Red Hat OpenShift version 4.12+
Red Hat OpenShift dynamic console plugin that leverages the NVIDIA GPU operator metrics and serves the respective console-extensions. Requires Red Hat OpenShift version 4.19+
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if version 0.2.6 is referenced in git tags, commits, or other files

echo "=== Searching for 0.2.6 in git tags ==="
git tag | grep -i '0\.2\.6' || echo "No tag found for 0.2.6"

echo -e "\n=== Searching for 0.2.6 in commit messages ==="
git log --all --oneline | grep -i '0\.2\.6' || echo "No commit message references 0.2.6"

echo -e "\n=== Searching for 0.2.6 in Chart.yaml history ==="
git log --all -p -- deployment/console-plugin-nvidia-gpu/Chart.yaml | grep -C2 '0\.2\.6' || echo "No 0.2.6 found in Chart.yaml history"

echo -e "\n=== Searching for 0.2.6 in other files ==="
rg -n '0\.2\.6' --type yaml --type md || echo "No other references to 0.2.6"

Repository: rh-ecosystem-edge/console-plugin-nvidia-gpu

Length of output: 418


🏁 Script executed:

# Also check current Chart.yaml and README
echo "=== Current Chart.yaml content ==="
cat deployment/console-plugin-nvidia-gpu/Chart.yaml

echo -e "\n=== Searching for version references in README ==="
fd -e md . | xargs rg -n '0\.2\.[0-9]|version|OpenShift' | head -40

Repository: rh-ecosystem-edge/console-plugin-nvidia-gpu

Length of output: 1632


Address version discrepancy and breaking change strategy.

Version 0.2.6 is documented in the README as supporting OpenShift 4.12–4.18, but does not exist in git history (no tags, commits, or Chart.yaml records). Simultaneously, Chart.yaml has been updated from 0.2.5 directly to 0.3.0 with a requirement for OpenShift 4.19+. This creates a problematic state:

  1. Undocumented version gap: 0.2.6 is listed in the README compatibility table but was never created. Clarify whether 0.2.6 is a planned maintenance release for 4.12–4.18 users, or if the README entry should be removed.
  2. Breaking change with minor version bump: Dropping support for OpenShift 4.12–4.18 is a breaking change that warrants a major version bump (e.g., 1.0.0) rather than a minor bump (0.3.0). If this is intentional, update the version strategy or provide migration guidance for affected users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant