Move installer-app to monorepo#2956
Move installer-app to monorepo#2956rawagner wants to merge 1 commit intoopenshift-assisted:masterfrom
Conversation
WalkthroughA new frontend application, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant AssistedInstallerApp
participant UIComponents
participant ApiClient
User->>Browser: Loads /apps/assisted-installer-app
Browser->>AssistedInstallerApp: Loads index.html and entry.ts
AssistedInstallerApp->>AssistedInstallerApp: bootstrap()
AssistedInstallerApp->>RootApp: Render RootApp
RootApp->>useInitApp: Initialize app (set base path, API interceptor)
useInitApp->>ApiClient: Set auth interceptor and base URL
RootApp->>UIComponents: Render UI components (routes, detail cards, etc.)
UIComponents->>ApiClient: Make API requests (with interceptor)
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
approved
rawagner
force-pushed
the
installer_app_monorepo
branch
from
7563438 to
13dc023
Compare
|
/hold just to make sure this won't get merged too soon :) |
openshift-ci
bot
added
the
do-not-merge/hold
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
♻️ Duplicate comments (7)
apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx (1)
1-3: Duplicate: missing@openshift-assisted/ui-lib/ocmdependency
This file faces the same import resolution error asServices.tsx. Please apply the same fix—install or alias@openshift-assisted/ui-lib/ocm.🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/TechnologyPreview.tsx (1)
1-3: Cannot resolve the UI library import
Same issue as in other wrappers:@openshift-assisted/ui-lib/ocmisn’t being found by the compiler or bundler. Please follow the steps outlined in theNoPermissionsError.tsxreview to add the library as a dependency and configure the necessary TS path mappings and bundler alias.🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/ClusterStatus.tsx (1)
1-3: Cannot resolve the UI library import
Identical to other component wrappers: the import from@openshift-assisted/ui-lib/ocmis unresolved. Ensure the monorepo’sui-libis declared as a dependency and configure TS path mappings and the bundler alias accordingly.🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml (4)
10-11: Fix YAML annotation formatting foron-cel-expression
Duplicate of push pipeline comment: the annotation is split across two lines. Consolidate into one line or use a block scalar.
26-29: Parameterize the hard-codedoutput-imagevalue
Duplicate of push pipeline comment: consider making the registry, namespace, and component configurable instead of hard-coding.
114-115: Add missingtypeto theimage-expires-afterparameter
Duplicate of push pipeline comment: theimage-expires-afterparam lacks atypedeclaration. Addtype: string.
33-539: Extract sharedpipelineSpecinto a reusablePipelineCR
Duplicate of push pipeline comment: factor out the common spec into aPipelineand reference viapipelineRef.
🧹 Nitpick comments (13)
apps/assisted-installer-app/config/empty.js (1)
1-2: Unify module format to ESM
The codebase is using ESM (see rootpackage.json"type": "module"), but this file uses CommonJS. For consistency and future-proofing, consider switching to an ESM export:- // Used as an empty module to save bundle size - module.exports = {}; + // Used as an empty module to save bundle size + export default {};apps/assisted-installer-app/README.md (2)
3-4: Clarify “stage stable server” phrasing
The term “stage stable server” may be confusing—consider rewording to clearly state the environment, e.g., “Install npm dependencies and start the development server” or specifying “staging”.
10-11: Format and verify the application URL
Consider wrapping the URL in Markdown link syntax for readability and confirm it’s the correct staging endpoint:Visit [https://stage.foo.redhat.com:1337/openshift/assisted-installer-app](https://stage.foo.redhat.com:1337/openshift/assisted-installer-app)apps/assisted-installer-app/src/bootstrap.ts (1)
5-8: Add a null check for the root DOM element
Currently, you’re using a non-null cast (as HTMLElement) which will throw if the element is missing. It’s safer to explicitly handle the case wheregetElementById('root')returnsnulland fail fast with a clear message:function bootstrap() { - const root = createRoot(document.getElementById('root') as HTMLElement); + const rootElement = document.getElementById('root'); + if (!rootElement) { + throw new Error("Root element with id 'root' not found"); + } + const root = createRoot(rootElement); root.render(React.createElement(RootApp)); }apps/assisted-installer-app/src/entry.ts (1)
1-4: Reconsider@ts-ignoreand handle dynamic import errors
Suppressing type checks may mask genuine issues. Instead, consider declaring the module or fixing typings. Also, sinceimport('./bootstrap')returns a promise, add a.catch()to log bootstrap failures:-// @ts-ignore -import('./bootstrap'); +import('./bootstrap') + .catch((err) => { + console.error("Failed to load bootstrap module:", err); + // Optionally render an error state to the user + });apps/assisted-installer-app/pr_check.sh (2)
11-11: Split declaration and export to avoid masking return values
ShellCheck SC2155 warns about inline exports. For clarity and to preserve exit codes, separate assignment and export:-export WORKSPACE=${WORKSPACE:-$APP_ROOT} +WORKSPACE=${WORKSPACE:-$APP_ROOT} +export WORKSPACE🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Annotate dynamic source for ShellCheck
ShellCheck SC1090 can’t follow non-constant sources. Suppress with:# shellcheck source=/dev/null source <(curl -sSL "$COMMON_BUILDER/src/frontend-build.sh")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/build_deploy.sh (2)
11-12: Split declaration and export to avoid masking return values
To address ShellCheck SC2155 warnings, assign then export separately:-export APP_ROOT=$(pwd) +APP_ROOT=$(pwd) +export APP_ROOT -export IMAGE_TAG=$(git rev-parse HEAD) +IMAGE_TAG=$(git rev-parse HEAD) +export IMAGE_TAG🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Annotate dynamic source for ShellCheck
Suppress SC1090 for dynamic sourcing by adding:# shellcheck source=/dev/null source <(curl -sSL "$COMMON_BUILDER/src/frontend-build.sh")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/src/i18n.ts (1)
5-11: Consider using dynamic locale for date formattingThe date formatter uses 'default' as the locale, which may not match the user's locale preference. Consider using the current i18n language or user's locale for better localization.
-const dateTimeFormatter = new Intl.DateTimeFormat('default', { +const getDateTimeFormatter = (locale = 'en') => new Intl.DateTimeFormat(locale, { month: 'short', day: 'numeric', hour: 'numeric', minute: 'numeric', year: 'numeric', });Then update line 41 to use this function:
-output = dateTimeFormatter.format(value); +output = getDateTimeFormatter(lng).format(value);apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx (1)
13-24: Consider adding error boundariesThe component initializes the app and immediately uses the result. If initialization fails, the component will render with potentially invalid data.
Consider adding error handling:
const AssistedInstallerDetailCard: React.FC<DetailsCardProps> = (props) => { + try { useInitApp(); const { chromeHistory } = useChrome(); return ( <AIDetailsCard history={chromeHistory as unknown as HistoryRouterProps['history']} allEnabledFeatures={{}} basename="/openshift" {...props} /> ); + } catch (error) { + console.error('Failed to initialize Assisted Installer Detail Card:', error); + return <div>Failed to load Assisted Installer. Please try refreshing the page.</div>; + } };docs/DEVELOPMENT.md (1)
56-56: Fix grammatical issue in package manager referenceThe phrase "package managers shims" has a grammatical issue with the plural form.
-1. Set up the package managers shims and install the project dependencies: +1. Set up the package manager shims and install the project dependencies:🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml (1)
25-29: Parameterize the hard-codedoutput-imagevalue
Theoutput-imageparameter is hard-coded to a specific registry and path. To support different environments and avoid duplication, consider introducing top-level parameters (e.g.,registry,namespace,component) or environment variables.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
.gitignore(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml(1 hunks)apps/assisted-installer-app/README.md(1 hunks)apps/assisted-installer-app/build_deploy.sh(1 hunks)apps/assisted-installer-app/config/empty.js(1 hunks)apps/assisted-installer-app/config/jest.setup.js(1 hunks)apps/assisted-installer-app/deploy/frontend.yaml(1 hunks)apps/assisted-installer-app/fec.config.js(1 hunks)apps/assisted-installer-app/jest.config.js(1 hunks)apps/assisted-installer-app/package.json(1 hunks)apps/assisted-installer-app/pr_check.sh(1 hunks)apps/assisted-installer-app/src/bootstrap.ts(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/ClusterStatus.tsx(1 hunks)apps/assisted-installer-app/src/components/ExternalLink.tsx(1 hunks)apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx(1 hunks)apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx(1 hunks)apps/assisted-installer-app/src/components/NoPermissionsError.tsx(1 hunks)apps/assisted-installer-app/src/components/RootApp.tsx(1 hunks)apps/assisted-installer-app/src/components/Services.tsx(1 hunks)apps/assisted-installer-app/src/components/TechnologyPreview.tsx(1 hunks)apps/assisted-installer-app/src/entry.ts(1 hunks)apps/assisted-installer-app/src/hooks/useInitApp.ts(1 hunks)apps/assisted-installer-app/src/i18n.ts(1 hunks)apps/assisted-installer-app/src/index.html(1 hunks)apps/assisted-installer-app/tsconfig.json(1 hunks)docs/DEVELOPMENT.md(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/assisted-installer-app/src/components/RootApp.tsx (1)
apps/assisted-installer-app/src/hooks/useInitApp.ts (1)
useInitApp(36-44)
apps/assisted-installer-app/src/components/TechnologyPreview.tsx (1)
libs/ui-lib/lib/common/components/ui/TechnologyPreview.tsx (1)
TechnologyPreview(11-24)
apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx (1)
apps/assisted-installer-app/src/hooks/useInitApp.ts (1)
useInitApp(36-44)
🪛 GitHub Actions: Pull request
apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx
[error] 3-3: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 2-2: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
[error] 5-5: TS2307: Cannot find module '@openshift-assisted/ui-lib/build/@types/ocm/components/HostsClusterDetailTab/types' or its corresponding type declarations.
apps/assisted-installer-app/src/components/RootApp.tsx
[error] 3-3: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 2-2: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/Services.tsx
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/ExternalLink.tsx
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/ClusterStatus.tsx
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/NoPermissionsError.tsx
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/TechnologyPreview.tsx
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/hooks/useInitApp.ts
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx
[error] 3-3: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 2-2: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🪛 Shellcheck (0.10.0)
apps/assisted-installer-app/build_deploy.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/pr_check.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🪛 Biome (1.9.4)
apps/assisted-installer-app/src/i18n.ts
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🪛 LanguageTool
docs/DEVELOPMENT.md
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🔇 Additional comments (17)
apps/assisted-installer-app/config/jest.setup.js (1)
1-1:Details
✅ Verification successful
Importing Testing Library’s custom matchers is correct
This setup file extends Jest’s global environment withjest-dommatchers, enabling more expressive DOM assertions in your component tests.
Ensure that yourjest.config.jsincludes this file undersetupFilesAfterEnv(e.g.,<rootDir>/config/jest.setup.js).Run this to verify the reference:
🏁 Script executed:
#!/bin/bash # Verify that jest.setup.js is listed in setupFilesAfterEnv rg -C2 "setupFilesAfterEnv" -n apps/assisted-installer-app/jest.config.jsLength of output: 219
Importing Testing Library’s custom matchers is properly configured
Verified thatapps/assisted-installer-app/jest.config.jsincludes<rootDir>/config/jest.setup.jsinsetupFilesAfterEnv(line 15), so thejest-dommatchers are active in your tests. No further changes needed.apps/assisted-installer-app/README.md (1)
1-2: Header is clear
The title# assisted-installer-appis concise and follows repo conventions..gitignore (1)
46-46: Include assisted-installer-app in version control
Adding!apps/assisted-installer-appis necessary to track all new files under that directory. This matches the existing pattern for other apps.package.json (1)
44-44: Add start script for assisted-installer-app workspace
The newstart:assisted_installer_appscript aligns with the other workspace commands. Verify that the workspace package name inapps/assisted-installer-app/package.jsonis@openshift-assisted/assisted-installer-app, and ensure this script is documented indocs/DEVELOPMENT.md.apps/assisted-installer-app/src/index.html (1)
1-11: Verify HTML entry includes mounting point and scripts
Yourindex.htmlrelies on ESI includes for head and body snippets. Please confirm that those snippets:
- Inject a DOM container (e.g.,
<div id="root"></div>) for React to mount into.- Include the
<script type="module" src="/entry.ts"></script>(or equivalent) that bootstraps your React application.If they don’t, you’ll need to add fallback markup and script tags here so the app can actually initialize.
apps/assisted-installer-app/src/components/RootApp.tsx (1)
16-17: Align routing base path with initialization logic
useInitAppsets the route base path to/assisted-installer-app, butRoutesis configured withbasename="/openshift". This mismatch can break routing.Please confirm the intended base path and update either
basename(e.g., to/assisted-installer-app) or the initialization hook to use/openshift.apps/assisted-installer-app/jest.config.js (1)
22-37: The SWC configuration looks goodThe SWC configuration is well set up for transforming TypeScript and JSX/TSX files with appropriate parser and transform options.
apps/assisted-installer-app/package.json (3)
38-57: LGTM! Dev dependencies are well configuredThe testing and development tools are up-to-date, including modern versions of TypeScript, ESLint, and testing libraries. The setup with SWC for Jest will provide faster test execution.
17-17: Verify the cloud environment and UI environment configurationThe start script uses specific flags
--clouddotEnv=stage --uiEnv=stable. Ensure these are the correct values for your development environment.
26-26:Details
❓ Verification inconclusive
Consider specifying a more recent version constraint for axios
The current version constraint
"axios": ">=0.22.0 <1.0.0"may include older versions with known security vulnerabilities. Consider updating to the latest 1.x version which includes important security fixes.
🏁 Script executed:
#!/bin/bash # Check for known security advisories in axios version range npm audit --json | jq '.advisories[] | select(.module_name=="axios") | {title: .title, vulnerable_versions: .vulnerable_versions, recommendation: .recommendation}'Length of output: 572
🏁 Script executed:
#!/bin/bash # Generate a lockfile for npm audit npm install --package-lock-only # Check for known security advisories in axios after lockfile creation npm audit --json | jq '.advisories[] | select(.module_name=="axios") | {title: .title, vulnerable_versions: .vulnerable_versions, recommendation: .recommendation}'Length of output: 822
Consider updating axios to the latest 1.x release
The current constraint
"axios": ">=0.22.0 <1.0.0",still allows older 0.x versions that have had security advisories (SSRF and other fixes landed in 1.x). Please:
- Review the npm advisory database for axios versions <1.0.0.
- Update the constraint to the latest 1.x release, for example:
"axios": ">=1.5.0 <2.0.0"apps/assisted-installer-app/deploy/frontend.yaml (1)
7-29: Ensure consistent module and route configurationThe frontend configuration looks good overall. The paths and API versions are properly defined. Make sure the module ID, module path, and route pathname are consistent with the federation setup in your
fec.config.jsfile.docs/DEVELOPMENT.md (2)
62-67: Instructions for starting the app have been simplifiedThe simplified instructions for starting the assisted-installer-app align well with the monorepo structure, making it easier for developers to get started.
75-80: LGTM! Clear reference to external documentationClearly referencing the uhc-portal's README for environment setup is a good practice, avoiding duplication of documentation.
🧰 Tools
🪛 LanguageTool
[style] ~77-~77: Consider a more concise word here.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...(IN_ORDER_TO_PREMIUM)
apps/assisted-installer-app/fec.config.js (4)
4-4: Verify the app URL pathEnsure that the appUrl value
/openshift/assisted-installer-appis consistent with the route pathname defined in the frontend.yaml template.
15-15: LGTM! Well-documented SASS prefix configurationThe
sassPrefixconfiguration is well-documented with an explanation of when and why it's needed, which helps future developers understand the configuration.
26-65: Comprehensive module federation configurationThe module federation setup exposes all necessary components from their corresponding file paths. This configuration enables proper integration with other applications.
70-71:Details
❓ Verification inconclusive
Check backend proxy configuration
The proxy routes are configured to a local backend on port 8002. Verify that this is the correct port for the backend service in the development environment.
🏁 Script executed:
#!/bin/bash # Check if port 8002 is commonly used in the codebase grep -r "8002" --include="*.js" --include="*.ts" --include="*.json" --include="*.md" .Length of output: 65623
Verify assisted-installer-app proxy port
A search across the codebase (
grep -r "8002") shows only these two entries inapps/assisted-installer-app/fec.config.js:'/api/assisted-installer-app': { host: 'http://localhost:8002' }, '/apps/assisted-installer-app': { host: 'http://localhost:8002' },Please confirm that the assisted-installer-app backend in your development environment is indeed listening on port 8002. If it uses a different port, update these proxy targets accordingly.
| ``` | ||
| npm install | ||
| npm start | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align install/start instructions with Yarn monorepo
This repo uses Yarn v3 workspaces (see root package.json). Running npm install here may not work under PnP. Update to:
- ```bash
- npm install
- npm start
- ```
+ ```bash
+ yarn install
+ yarn start
+ ```🤖 Prompt for AI Agents
In apps/assisted-installer-app/README.md around lines 5 to 8, the install and
start instructions use npm commands which are incompatible with the Yarn v3
workspace setup of this repo. Replace the npm commands with the corresponding
Yarn commands by changing "npm install" to "yarn install" and "npm start" to
"yarn start". Also update the code block language tag to "bash" for consistency.
| import { Services } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export default Services; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Missing dependency or incorrect import path for @openshift-assisted/ui-lib/ocm
The build errors (Module not found / TS2307) indicate that this package isn’t installed or the path is wrong. Please ensure that:
@openshift-assisted/ui-lib/ocmis listed underdependenciesofapps/assisted-installer-app/package.json.- The package name and subpath match the published module.
- If using webpack/module-federation aliases, that they include this library.
Once the package is correctly installed and resolvable, this import will succeed.
Run:
to verify the dependency, and install it with:
🏁 Script executed:
grep -R '"@openshift-assisted/ui-lib"' -n apps/assisted-installer-app/package.jsonLength of output: 84
🏁 Script executed:
yarn workspace @openshift-assisted/assisted-installer-app add @openshift-assisted/ui-libLength of output: 824
🏁 Script executed:
# Search for the UI lib dependency in all package.json files
grep -R '"@openshift-assisted/ui-lib' -n . --include package.json
# Find all import statements referencing the UI lib in the codebase
rg "@openshift-assisted/ui-lib" -nLength of output: 6135
Add missing @openshift-assisted/ui-lib dependency to assisted-installer-app
It looks like apps/assisted-installer-app/package.json does not list @openshift-assisted/ui-lib, yet you’re importing from @openshift-assisted/ui-lib/ocm. Please:
- In
apps/assisted-installer-app/package.json, underdependencies, add:
"@openshift-assisted/ui-lib": "workspace:*" - Run:
yarn workspace @openshift-assisted/assisted-installer-app add @openshift-assisted/ui-lib
- Verify that imports like
import { Services } from '@openshift-assisted/ui-lib/ocm'resolve without errors.
This will ensure your build can find the ui-lib package and its ocm subpath.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/Services.tsx lines 1 to 3, the
import from '@openshift-assisted/ui-lib/ocm' fails because the package is
missing from the assisted-installer-app dependencies. To fix this, add
"@openshift-assisted/ui-lib": "workspace:*" under dependencies in
apps/assisted-installer-app/package.json, then run 'yarn workspace
@openshift-assisted/assisted-installer-app add @openshift-assisted/ui-lib' to
install it. After that, verify the import resolves correctly without build
errors.
| import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export type { FeaturesSupportsLevel as default }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Module resolution and default-export of a type
- The import from
@openshift-assisted/ui-lib/ocmis failing—ensure that the package is installed or aliased as above. - Using
export type { FeaturesSupportsLevel as default };only re-exports a type and will vanish at runtime, causingimport FeatureSupportsLevel from '...';to beundefined. If you need a runtime default export, change this to:
-import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm';
-import '../i18n';
-export type { FeaturesSupportsLevel as default };
+import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm';
+import '../i18n';
+export default FeaturesSupportsLevel;📝 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.
| import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm'; | |
| import '../i18n'; | |
| export type { FeaturesSupportsLevel as default }; | |
| import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm'; | |
| import '../i18n'; | |
| export default FeaturesSupportsLevel; |
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx lines 1
to 3, the import from '@openshift-assisted/ui-lib/ocm' is failing likely because
the package is not installed or not properly aliased; ensure the package is
installed and correctly configured in your module resolution. Also, the
statement 'export type { FeaturesSupportsLevel as default };' only re-exports a
type and does not provide a runtime default export, causing default imports to
be undefined. To fix this, replace the type-only export with a proper runtime
export, such as 'export default FeaturesSupportsLevel;' if the imported entity
is a value, or adjust your usage to named imports if it is purely a type.
| import { NoPermissionsError } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export default NoPermissionsError; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Cannot resolve the UI library import
The build errors indicate that @openshift-assisted/ui-lib/ocm cannot be found. You need to ensure the shared UI library is properly referenced in this app:
- Add
@openshift-assisted/ui-libtoapps/assisted-installer-app/package.jsondependencies (e.g.,"@openshift-assisted/ui-lib": "workspace:*"). - Configure TypeScript
pathsmapping and your Vite/Rollup alias so that@openshift-assisted/ui-lib/ocmpoints tolibs/ui-lib/src(or to the compileddistfolder). - Verify that
libs/ui-lib/package.jsonexports the./ocmsubpath if you intend to support@openshift-assisted/ui-lib/ocm.
Without these changes, both TS and the bundler will continue to report “module not found.”
I’ll inspect the app’s package.json, the UI lib’s exports, TS paths, and any bundler alias to confirm whether the import is resolvable.
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Checking assisted-installer-app dependencies ==="
grep -R "\"@openshift-assisted/ui-lib\"" -n apps/assisted-installer-app/package.json || echo "→ @openshift-assisted/ui-lib NOT declared"
echo "=== Checking ui-lib package.json exports ==="
grep -R "\"exports\"" -n libs/ui-lib/package.json || echo "→ No exports field in ui-lib package.json"
echo "=== Verifying ./ocm subpath in exports ==="
grep -R "\"\\.\\/ocm\"" -n libs/ui-lib/package.json || echo "→ ./ocm not exported"
echo "=== Root tsconfig.json paths mapping ==="
grep -R "\"paths\"" -n tsconfig.json || echo "→ No paths configured in root tsconfig"
echo "=== App tsconfig.json paths mapping ==="
grep -R "\"paths\"" -n apps/assisted-installer-app/tsconfig.json || echo "→ No paths in app tsconfig"
echo "=== Vite/Rollup alias settings ==="
grep -R "alias" -n vite.config.ts || echo "→ No alias block found in vite.config.ts"
grep -R "resolve.alias" -n vite.config.ts || echo "→ No resolve.alias in vite.config.ts"Length of output: 1535
Cannot resolve @openshift-assisted/ui-lib/ocm import
The import of NoPermissionsError from @openshift-assisted/ui-lib/ocm fails because the app isn’t set up to reference the UI library. Here’s what needs to be fixed:
• apps/assisted-installer-app/package.json
– Add @openshift-assisted/ui-lib to dependencies, for example:
json "dependencies": { "@openshift-assisted/ui-lib": "workspace:*", … }
• TypeScript config (root tsconfig.json or apps/assisted-installer-app/tsconfig.json)
– Add a paths mapping so TS can resolve the library source:
json { "compilerOptions": { "paths": { "@openshift-assisted/ui-lib/*": ["libs/ui-lib/src/*"] } } }
• Vite/Rollup config (vite.config.ts/vite.config.js or rollup.config.js)
– Define an alias pointing @openshift-assisted/ui-lib at the UI-lib source:
js import { resolve } from 'path'; export default defineConfig({ resolve: { alias: { '@openshift-assisted/ui-lib': resolve(__dirname, 'libs/ui-lib/src') } } });
• libs/ui-lib/package.json
– The ./ocm subpath export is already present, so no changes needed there.
After adding the dependency, TS path mapping, and bundler alias, both TypeScript and the build will successfully resolve @openshift-assisted/ui-lib/ocm.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/NoPermissionsError.tsx lines 1 to
3, the import of NoPermissionsError from '@openshift-assisted/ui-lib/ocm' fails
because the app does not properly reference the shared UI library. To fix this,
add '@openshift-assisted/ui-lib' to the dependencies in
apps/assisted-installer-app/package.json with version "workspace:*". Then,
update the TypeScript configuration (either root tsconfig.json or
apps/assisted-installer-app/tsconfig.json) to include a paths mapping that
points '@openshift-assisted/ui-lib/*' to 'libs/ui-lib/src/*'. Finally, configure
the Vite or Rollup bundler (in vite.config.ts or equivalent) to add an alias
mapping '@openshift-assisted/ui-lib' to the 'libs/ui-lib/src' directory. These
steps will enable both TypeScript and the bundler to resolve the import
correctly.
| { | ||
| "extends": "@tsconfig/vite-react/tsconfig.json", | ||
| "include": ["src", "vite.config.ts"], | ||
| "references": [ | ||
| { | ||
| "path": "../../libs/ui-lib" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing compilerOptions for module aliasing
You’ve added a project reference to libs/ui-lib, but you also need to tell TS (and Vite) how to resolve the package imports. Consider extending your config:
{
"extends": "@tsconfig/vite-react/tsconfig.json",
+ "compilerOptions": {
+ "baseUrl": ".",
+ "paths": {
+ "@openshift-assisted/ui-lib/*": ["../../libs/ui-lib/src/*"]
+ }
+ },
"include": ["src", "vite.config.ts"],
"references": [
{ "path": "../../libs/ui-lib" }
]
}This ensures any import under @openshift-assisted/ui-lib/ocm (and other subpaths) resolves to your local library source.
📝 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.
| { | |
| "extends": "@tsconfig/vite-react/tsconfig.json", | |
| "include": ["src", "vite.config.ts"], | |
| "references": [ | |
| { | |
| "path": "../../libs/ui-lib" | |
| } | |
| ] | |
| } | |
| { | |
| "extends": "@tsconfig/vite-react/tsconfig.json", | |
| "compilerOptions": { | |
| "baseUrl": ".", | |
| "paths": { | |
| "@openshift-assisted/ui-lib/*": ["../../libs/ui-lib/src/*"] | |
| } | |
| }, | |
| "include": ["src", "vite.config.ts"], | |
| "references": [ | |
| { | |
| "path": "../../libs/ui-lib" | |
| } | |
| ] | |
| } |
🤖 Prompt for AI Agents
In apps/assisted-installer-app/tsconfig.json lines 1 to 9, you have added a
project reference to libs/ui-lib but did not configure compilerOptions for
module aliasing. To fix this, add a "compilerOptions" section with "paths"
mapping the package imports (e.g., "@openshift-assisted/ui-lib/ocm/*") to the
local source paths of the library. This will enable TypeScript and Vite to
correctly resolve imports from the referenced library.
| @@ -0,0 +1,26 @@ | |||
| import * as React from 'react'; | |||
| import { AssistedInstallerDetailCard as AIDetailsCard } from '@openshift-assisted/ui-lib/ocm'; | |||
There was a problem hiding this comment.
Resolve missing module dependency
The pipeline is failing because it cannot find the @openshift-assisted/ui-lib/ocm module. This is the same issue as in useInitApp.ts and needs to be addressed.
The import of AssistedInstallerDetailCard is failing due to the missing module. Ensure the package is properly installed and configured in your project.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 2-2: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx at
line 2, the import of AssistedInstallerDetailCard from
'@openshift-assisted/ui-lib/ocm' fails due to a missing module dependency. To
fix this, ensure that the '@openshift-assisted/ui-lib' package is installed in
your project by running the appropriate package manager command (e.g., npm
install or yarn add). Also verify that the package version includes the 'ocm'
export and that your project configuration supports this module resolution.
| spec: | ||
| envName: ${ENV_NAME} | ||
| title: Assisted Installer | ||
| deploymentRepo: https://github.com/openshift-assisted/assisted-installer-app |
There was a problem hiding this comment.
Update the deployment repository URL
The deploymentRepo URL still points to a standalone repository: https://github.com/openshift-assisted/assisted-installer-app. Since the app is being moved to a monorepo, this URL should be updated to reflect the new repository structure.
- deploymentRepo: https://github.com/openshift-assisted/assisted-installer-app
+ deploymentRepo: https://github.com/openshift-assisted/assisted-installer-ui📝 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.
| deploymentRepo: https://github.com/openshift-assisted/assisted-installer-app | |
| deploymentRepo: https://github.com/openshift-assisted/assisted-installer-ui |
🤖 Prompt for AI Agents
In apps/assisted-installer-app/deploy/frontend.yaml at line 14, update the
deploymentRepo URL from the old standalone repository to the new monorepo URL
that reflects the current repository structure. Replace the existing URL with
the correct monorepo path to ensure it points to the right source location.
| pipelinesascode.tekton.dev/max-keep-runs: "3" | ||
| pipelinesascode.tekton.dev/on-cel-expression: event == "push" && target_branch | ||
| == "main" |
There was a problem hiding this comment.
Fix YAML annotation formatting for on-cel-expression
The pipelinesascode.tekton.dev/on-cel-expression annotation is split across two lines, which will break YAML parsing. Consolidate into a single line or use a block scalar.
Example:
pipelinesascode.tekton.dev/on-cel-expression: >
event == "push" && target_branch == "main"🤖 Prompt for AI Agents
In
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
around lines 8 to 10, the annotation
pipelinesascode.tekton.dev/on-cel-expression is incorrectly split across two
lines, causing YAML parsing errors. Fix this by consolidating the entire
expression into a single line or use a block scalar style (| or >) to properly
format the multiline string, ensuring the expression remains intact and valid
YAML.
| finally: | ||
| - name: show-sbom | ||
| params: | ||
| - name: IMAGE_URL | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: show-sbom | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-show-sbom:0.1@sha256:002f7c8c1d2f9e09904035da414aba1188ae091df0ea9532cd997be05e73d594 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| - name: show-summary | ||
| params: | ||
| - name: pipelinerun-name | ||
| value: $(context.pipelineRun.name) | ||
| - name: git-url | ||
| value: $(tasks.clone-repository.results.url)?rev=$(tasks.clone-repository.results.commit) | ||
| - name: image-url | ||
| value: $(params.output-image) | ||
| - name: build-task-status | ||
| value: $(tasks.build-container.status) | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: summary | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-summary:0.2@sha256:76075b709fa06ed824cbc84f41448b397b85bfde1cf9809395ba6d286f5b7cbd | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| params: | ||
| - description: Source Repository URL | ||
| name: git-url | ||
| type: string | ||
| - default: "" | ||
| description: Revision of the Source Repository | ||
| name: revision | ||
| type: string | ||
| - description: Fully Qualified Output Image | ||
| name: output-image | ||
| type: string | ||
| - default: . | ||
| description: Path to the source code of an application's component from where | ||
| to build image. | ||
| name: path-context | ||
| type: string | ||
| - default: Dockerfile | ||
| description: Path to the Dockerfile inside the context specified by parameter | ||
| path-context | ||
| name: dockerfile | ||
| type: string | ||
| - default: "false" | ||
| description: Force rebuild image | ||
| name: rebuild | ||
| type: string | ||
| - default: "false" | ||
| description: Skip checks against built image | ||
| name: skip-checks | ||
| type: string | ||
| - default: "false" | ||
| description: Execute the build with network isolation | ||
| name: hermetic | ||
| type: string | ||
| - default: "" | ||
| description: Build dependencies to be prefetched by Cachi2 | ||
| name: prefetch-input | ||
| type: string | ||
| - default: "false" | ||
| description: Java build | ||
| name: java | ||
| type: string | ||
| - default: "" | ||
| description: Image tag expiration time, time values could be something like | ||
| 1h, 2d, 3w for hours, days, and weeks, respectively. | ||
| name: image-expires-after | ||
| - default: "false" | ||
| description: Build a source image. | ||
| name: build-source-image | ||
| type: string | ||
| - default: [] | ||
| description: Array of --build-arg values ("arg=value" strings) for buildah | ||
| name: build-args | ||
| type: array | ||
| - default: "" | ||
| description: Path to a file with build arguments for buildah, see https://www.mankier.com/1/buildah-build#--build-arg-file | ||
| name: build-args-file | ||
| type: string | ||
| results: | ||
| - description: "" | ||
| name: IMAGE_URL | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - description: "" | ||
| name: IMAGE_DIGEST | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - description: "" | ||
| name: CHAINS-GIT_URL | ||
| value: $(tasks.clone-repository.results.url) | ||
| - description: "" | ||
| name: CHAINS-GIT_COMMIT | ||
| value: $(tasks.clone-repository.results.commit) | ||
| tasks: | ||
| - name: init | ||
| params: | ||
| - name: image-url | ||
| value: $(params.output-image) | ||
| - name: rebuild | ||
| value: $(params.rebuild) | ||
| - name: skip-checks | ||
| value: $(params.skip-checks) | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: init | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-init:0.2@sha256:7a24924417260b7094541caaedd2853dc8da08d4bb0968f710a400d3e8062063 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| - name: clone-repository | ||
| params: | ||
| - name: url | ||
| value: $(params.git-url) | ||
| - name: revision | ||
| value: $(params.revision) | ||
| runAfter: | ||
| - init | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: git-clone | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-git-clone:0.1@sha256:3ced9a6b9d8520773d3ffbf062190515a362ecda11e72f56e38e4dd980294b57 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(tasks.init.results.build) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| workspaces: | ||
| - name: output | ||
| workspace: workspace | ||
| - name: basic-auth | ||
| workspace: git-auth | ||
| - name: prefetch-dependencies | ||
| params: | ||
| - name: input | ||
| value: $(params.prefetch-input) | ||
| runAfter: | ||
| - clone-repository | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: prefetch-dependencies | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-prefetch-dependencies:0.2@sha256:89fb1850fdfa76eb9cc9d38952cb637f3b44108bf9b53cad5ed3716e17c53678 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.prefetch-input) | ||
| operator: notin | ||
| values: | ||
| - "" | ||
| workspaces: | ||
| - name: source | ||
| workspace: workspace | ||
| - name: git-basic-auth | ||
| workspace: git-auth | ||
| - name: build-container | ||
| params: | ||
| - name: IMAGE | ||
| value: $(params.output-image) | ||
| - name: DOCKERFILE | ||
| value: $(params.dockerfile) | ||
| - name: CONTEXT | ||
| value: $(params.path-context) | ||
| - name: HERMETIC | ||
| value: $(params.hermetic) | ||
| - name: PREFETCH_INPUT | ||
| value: $(params.prefetch-input) | ||
| - name: IMAGE_EXPIRES_AFTER | ||
| value: $(params.image-expires-after) | ||
| - name: COMMIT_SHA | ||
| value: $(tasks.clone-repository.results.commit) | ||
| - name: BUILD_ARGS | ||
| value: | ||
| - $(params.build-args[*]) | ||
| - name: BUILD_ARGS_FILE | ||
| value: $(params.build-args-file) | ||
| runAfter: | ||
| - prefetch-dependencies | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: buildah | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-buildah:0.4@sha256:c777fdb0947aff3e4ac29a93ed6358c6f7994e6b150154427646788ec773c440 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(tasks.init.results.build) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| workspaces: | ||
| - name: source | ||
| workspace: workspace | ||
| - name: build-source-image | ||
| params: | ||
| - name: BINARY_IMAGE | ||
| value: $(params.output-image) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: source-build | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-source-build:0.2@sha256:11029fa30652154f772b44132f8a116382c136a6223e8f9576137f99b9901dcb | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(tasks.init.results.build) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| - input: $(params.build-source-image) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: deprecated-base-image-check | ||
| params: | ||
| - name: IMAGE_URL | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: IMAGE_DIGEST | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: deprecated-image-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-deprecated-image-check:0.5@sha256:ecd33669676b3a193ff4c2c6223cb912cc1b0cf5cc36e080eaec7718500272cf | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: clair-scan | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: clair-scan | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-clair-scan:0.2@sha256:878ae247ffc58d95a9ac68e4d658ef91ef039363e03e65a386bc0ead02d9d7d8 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: ecosystem-cert-preflight-checks | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: ecosystem-cert-preflight-checks | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-ecosystem-cert-preflight-checks:0.2@sha256:1157c6ac9805af8b8874e4b8d68d2403d99e1c007f63623566b5d848b27c1826 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: sast-snyk-check | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| runAfter: | ||
| - clone-repository | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-snyk-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-snyk-check:0.4@sha256:0d22dbaa528c8edf59aafab3600a0537b5408b80a4f69dd9cb616620795ecdc8 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: clamav-scan | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: clamav-scan | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-clamav-scan:0.2@sha256:24182598bf5161c4007988a7236e240f361c77a0a9b6973a6712dbae50d296bc | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: sast-coverity-check | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: IMAGE | ||
| value: $(params.output-image) | ||
| - name: DOCKERFILE | ||
| value: $(params.dockerfile) | ||
| - name: CONTEXT | ||
| value: $(params.path-context) | ||
| - name: HERMETIC | ||
| value: $(params.hermetic) | ||
| - name: PREFETCH_INPUT | ||
| value: $(params.prefetch-input) | ||
| - name: IMAGE_EXPIRES_AFTER | ||
| value: $(params.image-expires-after) | ||
| - name: COMMIT_SHA | ||
| value: $(tasks.clone-repository.results.commit) | ||
| - name: BUILD_ARGS | ||
| value: | ||
| - $(params.build-args[*]) | ||
| - name: BUILD_ARGS_FILE | ||
| value: $(params.build-args-file) | ||
| runAfter: | ||
| - coverity-availability-check | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-coverity-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-coverity-check:0.3@sha256:d7d9682b679c98ffe4633dd24b62cafb0799264d459cf2ab9fa1593e1a42bfdf | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - input: $(tasks.coverity-availability-check.results.STATUS) | ||
| operator: in | ||
| values: | ||
| - success | ||
| workspaces: | ||
| - name: source | ||
| workspace: workspace | ||
| - name: coverity-availability-check | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: coverity-availability-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-coverity-availability-check:0.2@sha256:8b58c4fae00c0dfe3937abfb8a9a61aa3c408cca4278b817db53d518428d944e | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: sast-shell-check | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-shell-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-shell-check:0.1@sha256:188a4f6a582ac43d4de46c3998ded3c2a8ee237fb0604d90559a3b6e0aa62b0f | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: sast-unicode-check | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-unicode-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-unicode-check:0.2@sha256:e4a5215b45b1886a185a9db8ab392f8440c2b0848f76d719885637cf8d2628ed | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: apply-tags | ||
| params: | ||
| - name: IMAGE | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: apply-tags | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-apply-tags:0.1@sha256:3f89ba89cacf8547261b5ce064acce81bfe470c8ace127794d0e90aebc8c347d | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| - name: rpms-signature-scan | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: rpms-signature-scan | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-rpms-signature-scan:0.2@sha256:297c2d8928aa3b114fcb1ba5d9da8b10226b68fed30706e78a6a5089c6cd30e3 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| workspaces: | ||
| - name: workspace | ||
| - name: git-auth | ||
| optional: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract shared pipelineSpec into a reusable Pipeline CR
The push‐triggered PipelineRun duplicates almost the entire pipelineSpec found in the pull‐request PipelineRun. To reduce maintenance overhead and ensure consistency, define a standalone Pipeline resource (e.g., assisted-installer-app-pipeline.yaml) and reference it via:
spec:
pipelineRef:
name: assisted-installer-app-pipelineThis centralizes changes and keeps PipelineRun definitions concise.
🤖 Prompt for AI Agents
In
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
around lines 31 to 535, the PipelineRun currently duplicates the entire
pipelineSpec which is also present in the pull-request PipelineRun. To fix this,
extract the shared pipelineSpec into a separate reusable Pipeline custom
resource definition (CR) file, for example named
assisted-installer-app-pipeline.yaml. Then, update this PipelineRun to reference
that Pipeline by replacing the pipelineSpec with a pipelineRef section
specifying the Pipeline's name. This will centralize the pipeline definition and
simplify maintenance.
| - default: "" | ||
| description: Image tag expiration time, time values could be something like | ||
| 1h, 2d, 3w for hours, days, and weeks, respectively. | ||
| name: image-expires-after | ||
| - default: "false" |
There was a problem hiding this comment.
Add missing type to the image-expires-after parameter
In the pipelineSpec.params section, the image-expires-after entry has a default value but no type. Tekton requires each param to declare its type (e.g., string).
- - default: ""
- description: Image tag expiration time...
- name: image-expires-after
+ - default: ""
+ description: Image tag expiration time...
+ name: image-expires-after
+ type: string📝 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.
| - default: "" | |
| description: Image tag expiration time, time values could be something like | |
| 1h, 2d, 3w for hours, days, and weeks, respectively. | |
| name: image-expires-after | |
| - default: "false" | |
| - default: "" | |
| description: Image tag expiration time, time values could be something like | |
| 1h, 2d, 3w for hours, days, and weeks, respectively. | |
| name: image-expires-after | |
| type: string | |
| - default: "false" |
🤖 Prompt for AI Agents
In
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
around lines 108 to 112, the parameter `image-expires-after` is missing the
required `type` field. Add `type: string` to this parameter to comply with
Tekton's requirement that each parameter must declare its type.
There was a problem hiding this comment.
Actionable comments posted: 21
♻️ Duplicate comments (6)
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml (6)
4-11: Multiline CEL expression may be misinterpreted
Same as PR pipeline: theon-cel-expressionis split. Use a folded block for safety.
31-70: Final tasks for SBOM and summary are well-defined
Identical to the pull-request pipeline; looks good.
71-126: Review pipeline parameter definitions
Repeat of PR pipeline feedback: add explicittype: stringforimage-expires-afterand remove or utilize the unusedjavaparameter.
124-136: Result naming style
Refer to the pull-request pipeline comment on using lowercase snake_case for results.
137-535: Consolidate tasks into reusable Pipeline
Tasks block is duplicated from the PR pipeline—extract to a sharedPipelineresource and reference viapipelineRef.
536-553: Verify service account and workspace secrets
Mirror the verification needed in the PR pipeline for the service account and git-auth secret.
🧹 Nitpick comments (32)
apps/assisted-installer-app/tsconfig.json (1)
3-3: Add anexcludefield to optimize type-checking.Without exclusions, TypeScript will scan
node_modules, build outputs, and any other files under the workspace, which can slow down incremental builds. Consider adding:"include": ["src", "vite.config.ts"], +"exclude": ["node_modules", "dist", "build"]apps/assisted-installer-app/README.md (2)
5-8: Specify fenced code block language
For better readability and to satisfy markdownlint (MD040), add a language identifier to the code fence, e.g.- ``` + ```sh npm install npm start<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 5-5: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> --- `10-10`: **Convert bare URL to markdown link** To address markdownlint (MD034), wrap the URL in `< >` or use link syntax, e.g.: ```markdown Visit [Assisted Installer App](https://stage.foo.redhat.com:1337/openshift/assisted-installer-app)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
10-10: Bare URL used
null(MD034, no-bare-urls)
package.json (1)
44-44: Inconsistent script naming convention
The new script"start:assisted_installer_app"uses underscores, whereas other scripts use colons and hyphens (e.g."start:assisted_ui"). For consistency and discoverability, consider renaming it to use the existing convention, e.g.:- "start:assisted_installer_app": "yarn workspace @openshift-assisted/assisted-installer-app start:federated", + "start:assisted-installer-app": "yarn workspace @openshift-assisted/assisted-installer-app start:federated",apps/assisted-installer-app/src/entry.ts (1)
1-3: Avoid suppressing TypeScript errors with ts-ignore
The// @ts-ignorebypasses all type checks on the dynamic import. Instead of ignoring, declare or configure the module in yourtsconfig.json(e.g. add"resolveJsonModule": trueor updatepaths) so the import can be correctly typed.apps/assisted-installer-app/src/bootstrap.ts (1)
6-7: Add a null check for the root container
Ifdocument.getElementById('root')returnsnull,createRootwill throw. Guard against this to improve robustness:const container = document.getElementById('root'); if (!container) { throw new Error('Root element not found'); } const root = createRoot(container);apps/assisted-installer-app/pr_check.sh (3)
10-12: Fix potential return value maskingThe APP_ROOT variable assignment uses command substitution inline with declaration, which could mask return values.
-export APP_ROOT=$(pwd) +export APP_ROOT +APP_ROOT=$(pwd)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
17-18: Consider making the build script localSourcing scripts directly from URLs introduces several risks:
- If the remote URL changes or is unavailable, builds will fail
- It's difficult to audit changes to the external script
- There's no version pinning
Consider copying the frontend-build.sh script into your repository or using a specific version tag in the URL.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
20-26: Add real tests instead of dummy test reportsThe script creates a dummy JUnit XML report rather than running actual tests.
Would you like help setting up actual Jest tests for your components that generate real JUnit reports? This would provide better verification of component functionality during PR checks.
apps/assisted-installer-app/build_deploy.sh (3)
11-13: Fix potential return value maskingBoth variable assignments use command substitution inline with declaration, which could mask return values.
-export APP_ROOT=$(pwd) -export IMAGE_TAG=$(git rev-parse HEAD) +export APP_ROOT +APP_ROOT=$(pwd) +export IMAGE_TAG +IMAGE_TAG=$(git rev-parse HEAD)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
17-18: Consider making the build script localAs with pr_check.sh, sourcing scripts directly from URLs introduces risks.
Consider copying the frontend-build.sh script into your repository or using a specific version tag.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
28-29: Uncomment or remove teardown_dockerThe
teardown_dockercommand is commented out. Either uncomment it if it's needed or remove it if it's not applicable.-# teardown_docker +teardown_dockerOr remove the line entirely if not needed.
docs/DEVELOPMENT.md (1)
56-56: Fix grammatical error in package managers description.The phrase "package managers shims" has a grammatical number issue. Consider using either "package manager shims" or "package manager's shims" instead.
-1. Set up the package managers shims and install the project dependencies: +1. Set up the package manager shims and install the project dependencies:🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
apps/assisted-installer-app/src/hooks/useInitApp.ts (2)
21-34: Consider adding error handling to the auth interceptor.The interceptor should handle potential errors during request transformation to prevent silent failures.
const buildAuthInterceptor = (): ((client: AxiosInstance) => AxiosInstance) => { const authInterceptor = (client: AxiosInstance): AxiosInstance => { - client.interceptors.request.use((config) => { + client.interceptors.request.use((config) => { const BASE_URL = config.baseURL || getBaseUrl(); const updatedConfig: AxiosRequestConfig = { ...config, url: `${BASE_URL}${config.url}`, }; return updatedConfig; - }); + }, (error) => { + console.error('Request interceptor error:', error); + return Promise.reject(error); + }); return client; }; return authInterceptor; };
16-16: Consider using React's useState for initialization tracking.Using a module-level variable to track initialization state might lead to unexpected behavior in certain scenarios (e.g., hot reloading, multiple instances). Consider using React's state management instead.
-let initialized = false; +import { useState } from 'react'; + +const useInitializedState = () => { + const [isInitialized, setIsInitialized] = useState(false); + return { + isInitialized, + setInitialized: () => setIsInitialized(true) + }; +};Then in the hook:
export const useInitApp = () => { - if (!initialized) { + const { isInitialized, setInitialized } = useInitializedState(); + if (!isInitialized) { // init only once - initialized = true; + setInitialized(); Config.setRouteBasePath('/assisted-installer-app'); // @ts-ignore Api.setAuthInterceptor(buildAuthInterceptor()); } };apps/assisted-installer-app/src/i18n.ts (2)
28-28: Consider disabling debug mode for production.The i18n configuration has
debug: true, which might not be appropriate for production builds. Consider making this configurable based on the environment.- debug: true, + debug: process.env.NODE_ENV !== 'production',
17-18: Address ESLint warning properly.Instead of disabling the ESLint rule, consider properly typing the imported translation to avoid type safety issues.
- // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - translation, + translation: translation as Record<string, string>,apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx (2)
18-18: Avoid unsafe type assertions.The double type assertion (
as unknown as) is potentially unsafe. Consider creating a proper type guard or converter function.- history={chromeHistory as unknown as HistoryRouterProps['history']} + history={convertToHistoryProps(chromeHistory)}And add this helper function:
function convertToHistoryProps(chromeHistory: any): HistoryRouterProps['history'] { // Add validation logic here if needed return chromeHistory; }
19-20: Consider making hardcoded values configurable.The empty object for
allEnabledFeaturesand hardcoded value forbasenamemight limit flexibility. Consider making these configurable or importing them from a configuration file.+import { APP_BASENAME } from '../constants'; // ... - allEnabledFeatures={{}} - basename="/openshift" + allEnabledFeatures={process.env.ENABLED_FEATURES ? JSON.parse(process.env.ENABLED_FEATURES) : {}} + basename={APP_BASENAME}apps/assisted-installer-app/fec.config.js (4)
4-8: Ensure environment variables control debug and proxy settingsThe configuration has debug, proxy, and verbose proxy settings hardcoded to
true. For production deployments, these should ideally be controlled by environment variables to avoid debugging overhead in production environments.- debug: true, - useProxy: true, - proxyVerbose: true, + debug: process.env.DEBUG === 'true', + useProxy: process.env.USE_PROXY === 'true', + proxyVerbose: process.env.PROXY_VERBOSE === 'true',
25-25: Consider making the node_modules directory configurableThe
nodeModulesDirectoriespath is hardcoded to '../../node_modules' which assumes a specific directory structure. Consider making this configurable via environment variables to improve flexibility.- nodeModulesDirectories: '../../node_modules', + nodeModulesDirectories: process.env.NODE_MODULES_DIR || '../../node_modules',
66-68: Consider shared module configurationThe
excludeandsharedarrays are empty. For module federation, properly configuring shared dependencies can significantly improve loading performance and bundle size. Consider specifying commonly used libraries like React in the shared array.- exclude: [], - shared: [], + exclude: [], + shared: [ + { + react: { + singleton: true, + requiredVersion: "^18.2.0", + }, + "react-dom": { + singleton: true, + requiredVersion: "^18.2.0", + }, + "react-router-dom": { + singleton: true, + requiredVersion: "^5.3.3", + }, + }, + ],
70-72: Make API host configurable via environment variablesThe API and app routes are hardcoded to
http://localhost:8002. Consider making this configurable via environment variables to support different environments.- routes: { - '/api/assisted-installer-app': { host: 'http://localhost:8002' }, - '/apps/assisted-installer-app': { host: 'http://localhost:8002' } - } + routes: { + '/api/assisted-installer-app': { host: process.env.API_HOST || 'http://localhost:8002' }, + '/apps/assisted-installer-app': { host: process.env.APPS_HOST || 'http://localhost:8002' } + }apps/assisted-installer-app/jest.config.js (1)
23-24: Document the purpose of the jest_workaround pluginThe configuration includes a plugin named
jest_workaroundwithout explanation. Add documentation to explain its purpose and why it's necessary.experimental: { - plugins: [['jest_workaround', {}]], + plugins: [ + // This plugin addresses SWC compatibility issues with Jest + // See: https://github.com/swc-project/swc/issues/5059 + ['jest_workaround', {}] + ], },apps/assisted-installer-app/package.json (4)
6-7: Update Node.js and npm version requirementsThe engine requirements specify Node.js >=16.0.0 and npm >=7.0.0, which are becoming outdated. Consider updating to more recent LTS versions to ensure better security and feature support.
- "node": ">=16.0.0", - "npm": ">=7.0.0" + "node": ">=18.0.0", + "npm": ">=9.0.0"
17-18: Make environment parameters configurableThe start script has hardcoded environment values (
--clouddotEnv=stage --uiEnv=stable). Consider making these configurable via environment variables for greater flexibility across different environments.- "start": "HOT=true fec dev --clouddotEnv=stage --uiEnv=stable --config ../../node_modules/@redhat-cloud-services/frontend-components-config/bin/prod.webpack.config.js", + "start": "HOT=true fec dev --clouddotEnv=${CLOUD_ENV:-stage} --uiEnv=${UI_ENV:-stable} --config ../../node_modules/@redhat-cloud-services/frontend-components-config/bin/prod.webpack.config.js",
45-45: Remove @types/redux-logger if not using ReduxThe dependencies include
@types/redux-logger, but there's no corresponding redux or redux-logger dependency. If you're not using Redux, consider removing this type definition.- "@types/redux-logger": "^3.0.12",
32-32:Details
❓ Verification inconclusive
Consider upgrading to React Router v6
The project uses React Router DOM v5.3.3, but v6 has been available for some time with improved features and performance. Consider upgrading unless there are specific compatibility requirements with v5.
🌐 Web query:
What are the major differences between React Router v5 and v6?💡 Result:
React Router v6 introduces several significant changes compared to v5, offering a more streamlined and intuitive routing experience for React applications. Here are the major differences between these versions:
Core Component Changes
Switch vs Routes: In v6, the
<Switch>component has been replaced with<Routes>. This new component offers improved path matching capabilities and doesn't require specific ordering of routes[2][4].Route Declaration: The way routes are declared has changed fundamentally:
- V5 used
component={Component}orrenderprops- V6 uses an
elementprop that takes JSX directly:element={<Component />}[2][9]Exact Prop Removed: React Router v6 has eliminated the need for the
exactprop as routes now match exactly by default. The improved path matching algorithm ensures the best route is chosen without requiring explicit exact matching[1][2][9].Navigation and Redirects
Navigate vs Redirect: The
<Redirect>component from v5 has been replaced with<Navigate>in v6[1][4][9]. For example:// V5 <Redirect to="/news" /> // V6 <Navigate replace to="/news" />useNavigate vs useHistory: The
useHistoryhook has been replaced withuseNavigate, offering a cleaner API for navigation[7].NavLink Changes: The
activeClassNameprop used in v5 has been removed. Instead, v6 uses a function-basedclassNameprop that receives an object with anisActiveproperty to determine styling[4].Improved Features
Bundle Size Reduction: React Router v6 has a significantly smaller bundle size, approximately 60% smaller than v5, resulting in faster load times and better performance[3][7].
Nested Routes: V6 provides improved support for nested routes through the
<Outlet>component, making it easier to create nested UI structures[6][8].Relative Links: Unlike v5, v6 supports relative links that automatically incorporate the parent route's URL, eliminating the need for explicit path construction using
match.urlormatch.path[2][6].Path Pattern Matching: React Router v6 features an enhanced algorithm for path pattern matching, making route matching more intuitive and predictable[1][2].
Implementation Differences
No Order Dependencies: In v5, route matching was sequential with order dependencies, while v6 is fully declarative without order dependencies.
Simpler Nested Routes: V6 makes it easier to define and use nested routes, eliminating the need to use the parent's
match.pathto build child routes[2][8].Dynamic Parameters: While both versions support route parameters (like
/users/:id), v6 provides better hooks and utilities for accessing and working with these parameters[6].React Router v6 represents a significant evolution that brings more modern patterns to React routing, making code more predictable and easier to maintain while reducing bundle size and improving performance.
Citations:
- 1: https://dev.to/arunavamodak/react-router-v5-vs-v6-dp0
- 2: https://reactrouter.com/6.30.0/upgrading/v5
- 3: https://www.microverse.org/blog/react-router-guide-migrating-from-react-router-v5-to-react-router-v6
- 4: https://www.youtube.com/watch?v=Z3HZSHLRhTA
- 5: https://dev.to/arunavamodak/react-router-v5-vs-v6-dp0/comments
- 6: https://pieces.app/blog/react-router-v6-a-comprehensive-guide-to-page-routing-in-react
- 7: https://blog.logrocket.com/migrating-react-router-v6-guide/
- 8: https://dev.to/abhay_yt_52a8e72b213be229/understanding-react-router-v6-features-setup-and-best-practices-4ank
- 9: https://dev.to/shyamdadhaniya_89/difference-between-react-router-dom-v5-v6-2j2f
- 10: https://trio.dev/guide-to-react-router-v6/
Consider upgrading react-router-dom to v6
React Router v6 delivers a smaller bundle size (~60% smaller), enhanced nested-route support, relative links, and a more intuitive, declarative API. That said, upgrading from v5.3.3 will involve refactoring your routes and navigation logic:
- Replace
<Switch>with<Routes>- Change route declarations from
component={Component}orrender={…}toelement={<Component />}- Drop the
exactprop (v6 routes match exactly by default)- Swap
<Redirect to="…"/>for<Navigate replace to="…"/>- Use the
useNavigate()hook instead ofuseHistory()- Update any
<NavLink>usage: removeactiveClassNameand use a function-basedclassName={({ isActive }) => …}- Introduce
<Outlet>for nested routes instead of manual path concatenationPlease review your codebase for these patterns, plan and implement the necessary changes, and ensure thorough testing before bumping the dependency.
apps/assisted-installer-app/deploy/frontend.yaml (1)
36-36: Consider parameterizing the image repositoryThe default image repository is hardcoded to
quay.io/app-sre/assisted-installer-app. Consider making this configurable or updating it to align with your monorepo structure.- value: quay.io/app-sre/assisted-installer-app + value: ${IMAGE_REPOSITORY:-quay.io/app-sre/assisted-installer-app}apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml (3)
4-11: Multiline CEL expression may be misinterpreted
Thepipelinesascode.tekton.dev/on-cel-expressionannotation is split across two lines, which could introduce unintended newline or leading space. Consider using a YAML folded block for clarity, for example:pipelinesascode.tekton.dev/on-cel-expression: |- event == "pull_request" && target_branch == "main"
71-126: Review pipeline parameter definitions
- The
image-expires-afterparam underpipelineSpec.paramslacks an explicittype: string(it’s inferred, but declaring it improves readability).- The
javaparameter is declared but never consumed by any task—either integrate it into relevant tasks or remove it to avoid confusion.
127-139: Result naming style
Result namesCHAINS-GIT_URLandCHAINS-GIT_COMMITuse uppercase and hyphens. For consistency with Tekton naming conventions, consider lowercase snake_case (e.g.,chains_git_url).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
.gitignore(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml(1 hunks)apps/assisted-installer-app/README.md(1 hunks)apps/assisted-installer-app/build_deploy.sh(1 hunks)apps/assisted-installer-app/config/empty.js(1 hunks)apps/assisted-installer-app/config/jest.setup.js(1 hunks)apps/assisted-installer-app/deploy/frontend.yaml(1 hunks)apps/assisted-installer-app/fec.config.js(1 hunks)apps/assisted-installer-app/jest.config.js(1 hunks)apps/assisted-installer-app/package.json(1 hunks)apps/assisted-installer-app/pr_check.sh(1 hunks)apps/assisted-installer-app/src/bootstrap.ts(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/ClusterStatus.tsx(1 hunks)apps/assisted-installer-app/src/components/ExternalLink.tsx(1 hunks)apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx(1 hunks)apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx(1 hunks)apps/assisted-installer-app/src/components/NoPermissionsError.tsx(1 hunks)apps/assisted-installer-app/src/components/RootApp.tsx(1 hunks)apps/assisted-installer-app/src/components/Services.tsx(1 hunks)apps/assisted-installer-app/src/components/TechnologyPreview.tsx(1 hunks)apps/assisted-installer-app/src/entry.ts(1 hunks)apps/assisted-installer-app/src/hooks/useInitApp.ts(1 hunks)apps/assisted-installer-app/src/i18n.ts(1 hunks)apps/assisted-installer-app/src/index.html(1 hunks)apps/assisted-installer-app/tsconfig.json(1 hunks)docs/DEVELOPMENT.md(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
apps/assisted-installer-app/src/components/TechnologyPreview.tsx (1)
libs/ui-lib/lib/common/components/ui/TechnologyPreview.tsx (1)
TechnologyPreview(11-24)
apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx (2)
libs/ui-lib/lib/ocm/components/HostsClusterDetailTab/types.ts (1)
HostsClusterDetailTabProps(4-8)apps/assisted-installer-app/src/hooks/useInitApp.ts (1)
useInitApp(36-44)
apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx (1)
apps/assisted-installer-app/src/hooks/useInitApp.ts (1)
useInitApp(36-44)
apps/assisted-installer-app/src/components/RootApp.tsx (1)
apps/assisted-installer-app/src/hooks/useInitApp.ts (1)
useInitApp(36-44)
🪛 GitHub Actions: Pull request
apps/assisted-installer-app/src/components/TechnologyPreview.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/hooks/useInitApp.ts
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/NoPermissionsError.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/Services.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/ExternalLink.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/ClusterStatus.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 2-5: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or '@openshift-assisted/ui-lib/build/@types/ocm/components/HostsClusterDetailTab/types' or their corresponding type declarations.
apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 2-2: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
apps/assisted-installer-app/src/components/RootApp.tsx
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 2-2: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🪛 Biome (1.9.4)
apps/assisted-installer-app/src/i18n.ts
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🪛 LanguageTool
docs/DEVELOPMENT.md
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🪛 markdownlint-cli2 (0.17.2)
apps/assisted-installer-app/README.md
5-5: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
10-10: Bare URL used
null
(MD034, no-bare-urls)
🪛 Shellcheck (0.10.0)
apps/assisted-installer-app/pr_check.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/build_deploy.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🔇 Additional comments (26)
apps/assisted-installer-app/tsconfig.json (2)
2-2: Correct base config extension.Extending
@tsconfig/vite-react/tsconfig.jsonis appropriate for a Vite + React project and brings in sensible defaults (JSX support, strict mode, module resolution).
4-8:Details
✅ Verification successful
Verify project reference is correctly set up.
Project references require the referenced project to have
"composite": true(and typically"declaration": true) in its owntsconfig.json. Please confirm thatlibs/ui-lib/tsconfig.jsonis configured accordingly so incremental builds and type-checks work as expected.
🏁 Script executed:
#!/bin/bash # Verify that composite builds are enabled in the referenced project rg '"composite"\s*:\s*true' -n libs/ui-lib/tsconfig.json rg '"declaration"\s*:\s*true' -n libs/ui-lib/tsconfig.jsonLength of output: 171
Project reference configuration verified
The
libs/ui-lib/tsconfig.jsonalready includes both"composite": true, "declaration": trueso the project reference in
apps/assisted-installer-app/tsconfig.jsonis correctly set up. No further changes are needed..gitignore (1)
46-46: Include the new app directory in version control.
The addition of!apps/assisted-installer-appcorrectly overrides theapps/*ignore rule to track the new frontend application.apps/assisted-installer-app/src/index.html (1)
1-11: Basic HTML entry point added.
The staticindex.htmlcorrectly defines the document structure, setslangandcharset, and includes ESI directives for injecting the head and body snippets. This aligns with our deployment model.apps/assisted-installer-app/config/jest.setup.js (1)
1-1: Ensure@testing-library/jest-domis installed
This setup file importsjest-globalsfrom@testing-library/jest-dom. Confirm that@testing-library/jest-domis listed underdevDependenciesinpackage.jsonto prevent runtime errors.apps/assisted-installer-app/config/empty.js (1)
1-2: Empty placeholder module
Exporting an empty object to reduce bundle size is fine. Verify that any consumers of this module gracefully handle an empty configuration.apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx (1)
7-10: Component implements wrapper pattern correctlyThe component structure looks good. It initializes the app and wraps the library component with proper props forwarding.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
apps/assisted-installer-app/pr_check.sh (1)
7-7:Details
❓ Verification inconclusive
Verify component name consistency
The
COMPONENTis set to "assisted-installer" while the file is in "assisted-installer-app" directory. Ensure this is intentional and matches the expected configuration in app-interface.
🏁 Script executed:
#!/bin/bash # Check if there are any references to this component name in other files grep -r "assisted-installer" --include="*.yaml" --include="*.json" apps/assisted-installer-app/Length of output: 3387
🏁 Script executed:
#!/bin/bash sed -n '1,200p' apps/assisted-installer-app/pr_check.shLength of output: 1056
Verify COMPONENT name matches app-interface configuration
It looks like in
apps/assisted-installer-app/pr_check.sh(line 7) you’ve setexport COMPONENT="assisted-installer" export IMAGE="quay.io/app-sre/assisted-installer-app"However, all of your Tekton and deployment configs under
apps/assisted-installer-app/refer toassisted-installer-app(e.g. inpackage.json, YAML labels, image repo, etc.).Please confirm whether the
COMPONENTvalue should indeed be assisted-installer (and matches the application folder in app-interface), or whether it needs to be updated to assisted-installer-app for consistency with your image names and app-interface settings.– File:
apps/assisted-installer-app/pr_check.sh(line 7)
– Grep of config files shows only “assisted-installer-app” references under this directory.apps/assisted-installer-app/src/components/RootApp.tsx (1)
15-16: Consider passing actual feature flagsThe component passes an empty object for
allEnabledFeatures, which might disable important functionality.Verify whether any features should be enabled by default rather than passing an empty object.
apps/assisted-installer-app/build_deploy.sh (1)
7-7: Verify component name consistencySimilar to pr_check.sh, the
COMPONENTis set to "assisted-installer" while the file is in "assisted-installer-app" directory. Ensure this is intentional.docs/DEVELOPMENT.md (1)
56-80: LGTM! Documentation is now simplified.The integration instructions have been significantly streamlined, which aligns well with the PR objective of moving the installer-app to a monorepo structure. The workflow is now more straightforward with fewer steps and dependencies.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~77-~77: Consider a more concise word here.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...(IN_ORDER_TO_PREMIUM)
apps/assisted-installer-app/src/i18n.ts (1)
1-65: LGTM! Comprehensive i18n setup.The internationalization setup is well-structured with proper configuration for language support, fallbacks, and formatting. The implementation of custom formatters for dates and numbers is a nice touch.
🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx (1)
8-11: LGTM! Good use of TypeScript's Omit utility.The component properly uses TypeScript's
Omitutility type to define its props by excluding specific props from the wrapped component.apps/assisted-installer-app/fec.config.js (3)
15-15: Confirm SASS prefix configuration is necessaryThe commented guidance indicates this sassPrefix configuration is only needed when your appname includes dash characters. Since your app name does contain dashes, this is likely correct, but verify that this is the proper naming convention when testing your styles.
19-19: Verify Chrome configuration interception settingThe comment suggests changing
interceptChromeConfigtofalseafter your app is registered in configuration files, but it's already set tofalse. Confirm that this is the intended setting for your current integration phase.
27-65:Details
✅ Verification successful
Validate the existence of all exposed component files
The configuration exposes multiple React components. Ensure all these component files exist at the specified paths before deployment.
🏁 Script executed:
#!/bin/bash # Check if all exposed component files exist echo "Checking if all exposed component files exist:" COMPONENTS=( "src/components/RootApp.tsx" "src/components/TechnologyPreview.tsx" "src/components/NoPermissionsError.tsx" "src/components/ExternalLink.tsx" "src/components/AssistedInstallerDetailCard.tsx" "src/components/AssistedInstallerExtraDetailCard.tsx" "src/components/ClusterStatus.tsx" "src/components/FeatureSupportsLevel.tsx" "src/components/HostsClusterDetailTab.tsx" "src/components/Services.tsx" ) cd apps/assisted-installer-app for component in "${COMPONENTS[@]}"; do if [ -f "$component" ]; then echo "✅ $component exists" else echo "❌ $component does not exist" fi doneLength of output: 2450
Validation complete: all exposed component files exist
All files listed under the
exposessection inapps/assisted-installer-app/fec.config.jshave been verified at their specified paths. No further action is required.apps/assisted-installer-app/jest.config.js (1)
15-15:Details
✅ Verification successful
Verify that the Jest setup file exists
The configuration references a setup file at
<rootDir>/config/jest.setup.js. Ensure this file exists and contains the necessary test environment setup.
🏁 Script executed:
#!/bin/bash # Check if Jest setup file exists if [ -f "apps/assisted-installer-app/config/jest.setup.js" ]; then echo "✅ Jest setup file exists" echo "Content of the file:" cat "apps/assisted-installer-app/config/jest.setup.js" else echo "❌ Jest setup file does not exist" fiLength of output: 275
Confirmed: Jest setup file exists and imports necessary globals
The file
apps/assisted-installer-app/config/jest.setup.jsis present and correctly imports@testing-library/jest-dom/jest-globalsto set up the testing environment.apps/assisted-installer-app/deploy/frontend.yaml (1)
28-28: Ensure route pathname matches appUrl in fec.config.jsThe pathname
/openshift/assisted-installer-appshould match the appUrl in fec.config.js. Verify they are the same to ensure consistent routing.apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml (5)
1-3: Basic PipelineRun declaration is valid
apiVersion and kind are correct for Tekton v1.
12-18: Metadata labels and naming are consistent
The labels, name (assisted-installer-app-saas-main-on-pull-request), and namespace (assisted-installer-tenant) follow the project’s conventions.
20-32: Explicit PR image expiration set
You’ve setimage-expires-after: 5dfor pull-request builds to auto-clean images after five days. Confirm that 5 days is the intended TTL.
33-70: Final tasks for SBOM and summary are well-defined
Thefinallysection correctly invokesshow-sbomandshow-summary, ensuring SBOM generation and build summary are always executed.
537-556: Verify service account and workspace secrets
Ensure the ServiceAccountbuild-pipeline-assisted-installer-app-saas-mainexists in theassisted-installer-tenantnamespace, and that thegit_auth_secrettemplate variable is correctly populated in your Pipelines as Code setup so thegit-authworkspace can fetch private repos.apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml (3)
1-3: Basic PipelineRun declaration is valid
(apiVersion and kind are correct for push-triggered PipelineRun.)
12-18: Metadata labels and naming are consistent
assisted-installer-app-saas-main-on-pushcorrectly identifies the push-trigger pipeline.
20-30: Top-level params differ from PR pipeline
This run omitsimage-expires-after, so it will fall back to the pipeline default (""). Confirm that no TTL on push images is acceptable, or explicitly set if needed.
| import { Services } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export default Services; |
There was a problem hiding this comment.
Missing workspace dependency for @openshift-assisted/ui-lib
The import from @openshift-assisted/ui-lib/ocm fails with “module not found” because the app’s package.json does not list @openshift-assisted/ui-lib as a dependency under Yarn PnP. Please add it to apps/assisted-installer-app/package.json:
--- apps/assisted-installer-app/package.json
+++ apps/assisted-installer-app/package.json
@@
"dependencies": {
- },
+ "@openshift-assisted/ui-lib": "workspace:*"
+ },📝 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.
| import { Services } from '@openshift-assisted/ui-lib/ocm'; | |
| import '../i18n'; | |
| export default Services; | |
| --- apps/assisted-installer-app/package.json | |
| +++ apps/assisted-installer-app/package.json | |
| @@ | |
| - "dependencies": { | |
| - }, | |
| + "dependencies": { | |
| + "@openshift-assisted/ui-lib": "workspace:*" | |
| + }, |
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/Services.tsx lines 1 to 3, the
import from '@openshift-assisted/ui-lib/ocm' fails due to a missing workspace
dependency. To fix this, add '@openshift-assisted/ui-lib' as a dependency in the
apps/assisted-installer-app/package.json file under the dependencies section,
then run the package manager to update the workspace and resolve the module
correctly.
| import { AssistedInstallerExtraDetailCard } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export default AssistedInstallerExtraDetailCard; |
There was a problem hiding this comment.
Missing workspace dependency for @openshift-assisted/ui-lib
This file also imports from @openshift-assisted/ui-lib/ocm and suffers the same resolution error. Ensure @openshift-assisted/ui-lib is declared in the app’s package.json dependencies (see Services.tsx comment).
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In
apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
lines 1 to 3, the import from '@openshift-assisted/ui-lib/ocm' causes a
resolution error because '@openshift-assisted/ui-lib' is not declared as a
dependency. Fix this by adding '@openshift-assisted/ui-lib' to the dependencies
section of the app's package.json file to ensure proper module resolution.
| import { TechnologyPreview } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export default TechnologyPreview; |
There was a problem hiding this comment.
Critical: @openshift-assisted/ui-lib/ocm module not found
The import import { TechnologyPreview } from '@openshift-assisted/ui-lib/ocm'; is failing with TS2307 / module not found. This indicates the @openshift-assisted/ui-lib package isn’t installed or monorepo path resolution isn’t configured.
Raise a critical issue:
- Add the UI library as a dependency in
apps/assisted-installer-app/package.json(e.g.,"@openshift-assisted/ui-lib": "workspace:*"). - Ensure your root
package.jsonincludes workspaces forlibs/ui-lib. - Configure TypeScript path mappings in
tsconfig.jsonto resolve theocmentrypoint:
// tsconfig.json
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
+ "@openshift-assisted/ui-lib/ocm": ["../../libs/ui-lib/src/ocm"]
}
}
}📝 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.
| import { TechnologyPreview } from '@openshift-assisted/ui-lib/ocm'; | |
| import '../i18n'; | |
| export default TechnologyPreview; | |
| // tsconfig.json | |
| { | |
| "compilerOptions": { | |
| "baseUrl": ".", | |
| "paths": { | |
| "@openshift-assisted/ui-lib/ocm": ["../../libs/ui-lib/src/ocm"] | |
| } | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/TechnologyPreview.tsx lines 1 to
3, the import from '@openshift-assisted/ui-lib/ocm' fails due to missing module.
Fix this by adding "@openshift-assisted/ui-lib": "workspace:*" as a dependency
in apps/assisted-installer-app/package.json, ensure the root package.json
includes the libs/ui-lib workspace, and configure the tsconfig.json paths to
correctly resolve the 'ocm' entrypoint within the ui-lib package.
| import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export type { FeaturesSupportsLevel as default }; |
There was a problem hiding this comment.
Critical: @openshift-assisted/ui-lib/ocm module not found & type re-export pattern
The import import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm'; fails with TS2307. Resolve as in TechnologyPreview.tsx.
Additionally, the current default export of a type via export type { FeaturesSupportsLevel as default }; is uncommon. Consider a named type export instead:
- import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm';
- import '../i18n';
- export type { FeaturesSupportsLevel as default };
+ import '../i18n';
+ export type { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm';📝 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.
| import { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm'; | |
| import '../i18n'; | |
| export type { FeaturesSupportsLevel as default }; | |
| import '../i18n'; | |
| export type { FeaturesSupportsLevel } from '@openshift-assisted/ui-lib/ocm'; |
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx lines 1
to 3, the import from '@openshift-assisted/ui-lib/ocm' fails with TS2307 because
the module is not found. Fix this by adjusting the import path or method as done
in TechnologyPreview.tsx to correctly resolve the module. Also, replace the
uncommon default type export with a named type export for FeaturesSupportsLevel
to follow standard TypeScript export patterns.
| import { ExternalLink } from '@openshift-assisted/ui-lib/ocm'; | ||
| import '../i18n'; | ||
| export default ExternalLink; |
There was a problem hiding this comment.
Critical: @openshift-assisted/ui-lib/ocm module not found
The import import { ExternalLink } from '@openshift-assisted/ui-lib/ocm'; is failing (TS2307). Apply the same dependency and path fixes as in TechnologyPreview.tsx to ensure the UI library is available to the app.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-10: Module not found: Can't resolve '@openshift-assisted/ui-lib/ocm'. Import failure causes build error.
[error] 1-1: TS2307: Cannot find module '@openshift-assisted/ui-lib/ocm' or its corresponding type declarations.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/components/ExternalLink.tsx lines 1 to 3, the
import of ExternalLink from '@openshift-assisted/ui-lib/ocm' fails due to a
missing module. Fix this by updating the import path and dependencies exactly as
done in TechnologyPreview.tsx, ensuring the UI library is correctly referenced
and available to the app. Verify the module resolution and adjust any related
package.json or tsconfig paths if necessary.
| spec: | ||
| envName: ${ENV_NAME} | ||
| title: Assisted Installer | ||
| deploymentRepo: https://github.com/openshift-assisted/assisted-installer-app |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update deploymentRepo URL for monorepo structure
The deploymentRepo URL points to a standalone repository, but according to the PR description, you're moving to a monorepo structure. Update this URL to reflect the new repository structure.
- deploymentRepo: https://github.com/openshift-assisted/assisted-installer-app
+ deploymentRepo: https://github.com/openshift-assisted/assisted-installer-ui📝 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.
| deploymentRepo: https://github.com/openshift-assisted/assisted-installer-app | |
| deploymentRepo: https://github.com/openshift-assisted/assisted-installer-ui |
🤖 Prompt for AI Agents
In apps/assisted-installer-app/deploy/frontend.yaml at line 14, the
deploymentRepo URL is outdated and points to the old standalone repository.
Update the deploymentRepo URL to the new monorepo repository URL as described in
the PR to reflect the current repository structure.
| deploymentRepo: https://github.com/openshift-assisted/assisted-installer-app | ||
| frontend: | ||
| paths: | ||
| - /apps/assisted-installer-app |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify frontend path alignment with fec.config.js
The frontend path is set to /apps/assisted-installer-app, but in fec.config.js the appUrl is /openshift/assisted-installer-app. Ensure these paths are aligned or document why they differ.
🏁 Script executed:
#!/bin/bash
# Check for path consistency between frontend.yaml and fec.config.js
echo "Frontend path in deploy/frontend.yaml:"
grep -A 2 "frontend:" apps/assisted-installer-app/deploy/frontend.yaml | grep -A 1 "paths:"
echo "appUrl in fec.config.js:"
grep "appUrl:" apps/assisted-installer-app/fec.config.jsLength of output: 395
Align frontend deployment path with fec.config.js
There’s a mismatch between the route in your deployment manifest and the appUrl in fec.config.js, which can lead to routing or proxy issues. Please reconcile these values or document why they differ:
• In apps/assisted-installer-app/deploy/frontend.yaml (around line 17):
- - /apps/assisted-installer-app
+ - /openshift/assisted-installer-app• In apps/assisted-installer-app/fec.config.js:
appUrl: '/openshift/assisted-installer-app',Either update the frontend.yaml path to /openshift/assisted-installer-app or adjust appUrl to /apps/assisted-installer-app, and add a comment or documentation if there’s intentional divergence.
🤖 Prompt for AI Agents
In apps/assisted-installer-app/deploy/frontend.yaml at line 17, the frontend
path is set to /apps/assisted-installer-app, which conflicts with the appUrl
/openshift/assisted-installer-app in apps/assisted-installer-app/fec.config.js.
To fix this, update the path in frontend.yaml to
/openshift/assisted-installer-app to match fec.config.js, or alternatively,
change the appUrl in fec.config.js to /apps/assisted-installer-app.
Additionally, add a comment explaining the reason if the difference is
intentional.
| - v1 | ||
| image: ${IMAGE}:${IMAGE_TAG} | ||
| module: | ||
| manifestLocation: '/apps/assisted-installer-app/fed-mods.json' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the existence of fed-mods.json
The configuration references a manifest file at /apps/assisted-installer-app/fed-mods.json. Ensure this file exists or will be generated during the build process.
🏁 Script executed:
#!/bin/bash
# Check if fed-mods.json exists or is generated during build
if [ -f "apps/assisted-installer-app/fed-mods.json" ]; then
echo "✅ fed-mods.json exists"
else
echo "❌ fed-mods.json does not exist in the source"
echo "Checking if it's generated during build process..."
grep -r "fed-mods.json" apps/assisted-installer-app
fiLength of output: 439
Missing fed-mods.json in source or build output
The deployment config in apps/assisted-installer-app/deploy/frontend.yaml points at:
manifestLocation: '/apps/assisted-installer-app/fed-mods.json'
Our checks show:
- No
fed-mods.jsonfile in the repo underapps/assisted-installer-app/ - No build scripts or CI configs reference its generation
Please ensure one of the following before merging:
- Add the
apps/assisted-installer-app/fed-mods.jsonmanifest to source control - Or update your build pipeline to generate
fed-mods.jsonat/apps/assisted-installer-app/fed-mods.jsonprior to deployment
🤖 Prompt for AI Agents
In apps/assisted-installer-app/deploy/frontend.yaml at line 23, the
manifestLocation points to '/apps/assisted-installer-app/fed-mods.json', but
this file does not exist in the source or build output. To fix this, either add
the fed-mods.json manifest file to the source control at the specified path or
update the build pipeline to generate this file at
'/apps/assisted-installer-app/fed-mods.json' before deployment.
| tasks: | ||
| - name: init | ||
| params: | ||
| - name: image-url | ||
| value: $(params.output-image) | ||
| - name: rebuild | ||
| value: $(params.rebuild) | ||
| - name: skip-checks | ||
| value: $(params.skip-checks) | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: init | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-init:0.2@sha256:7a24924417260b7094541caaedd2853dc8da08d4bb0968f710a400d3e8062063 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| - name: clone-repository | ||
| params: | ||
| - name: url | ||
| value: $(params.git-url) | ||
| - name: revision | ||
| value: $(params.revision) | ||
| runAfter: | ||
| - init | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: git-clone | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-git-clone:0.1@sha256:3ced9a6b9d8520773d3ffbf062190515a362ecda11e72f56e38e4dd980294b57 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(tasks.init.results.build) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| workspaces: | ||
| - name: output | ||
| workspace: workspace | ||
| - name: basic-auth | ||
| workspace: git-auth | ||
| - name: prefetch-dependencies | ||
| params: | ||
| - name: input | ||
| value: $(params.prefetch-input) | ||
| runAfter: | ||
| - clone-repository | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: prefetch-dependencies | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-prefetch-dependencies:0.2@sha256:89fb1850fdfa76eb9cc9d38952cb637f3b44108bf9b53cad5ed3716e17c53678 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.prefetch-input) | ||
| operator: notin | ||
| values: | ||
| - "" | ||
| workspaces: | ||
| - name: source | ||
| workspace: workspace | ||
| - name: git-basic-auth | ||
| workspace: git-auth | ||
| - name: build-container | ||
| params: | ||
| - name: IMAGE | ||
| value: $(params.output-image) | ||
| - name: DOCKERFILE | ||
| value: $(params.dockerfile) | ||
| - name: CONTEXT | ||
| value: $(params.path-context) | ||
| - name: HERMETIC | ||
| value: $(params.hermetic) | ||
| - name: PREFETCH_INPUT | ||
| value: $(params.prefetch-input) | ||
| - name: IMAGE_EXPIRES_AFTER | ||
| value: $(params.image-expires-after) | ||
| - name: COMMIT_SHA | ||
| value: $(tasks.clone-repository.results.commit) | ||
| - name: BUILD_ARGS | ||
| value: | ||
| - $(params.build-args[*]) | ||
| - name: BUILD_ARGS_FILE | ||
| value: $(params.build-args-file) | ||
| runAfter: | ||
| - prefetch-dependencies | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: buildah | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-buildah:0.4@sha256:c777fdb0947aff3e4ac29a93ed6358c6f7994e6b150154427646788ec773c440 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(tasks.init.results.build) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| workspaces: | ||
| - name: source | ||
| workspace: workspace | ||
| - name: build-source-image | ||
| params: | ||
| - name: BINARY_IMAGE | ||
| value: $(params.output-image) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: source-build | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-source-build:0.2@sha256:11029fa30652154f772b44132f8a116382c136a6223e8f9576137f99b9901dcb | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(tasks.init.results.build) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| - input: $(params.build-source-image) | ||
| operator: in | ||
| values: | ||
| - "true" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: deprecated-base-image-check | ||
| params: | ||
| - name: IMAGE_URL | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: IMAGE_DIGEST | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: deprecated-image-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-deprecated-image-check:0.5@sha256:ecd33669676b3a193ff4c2c6223cb912cc1b0cf5cc36e080eaec7718500272cf | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: clair-scan | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: clair-scan | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-clair-scan:0.2@sha256:878ae247ffc58d95a9ac68e4d658ef91ef039363e03e65a386bc0ead02d9d7d8 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: ecosystem-cert-preflight-checks | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: ecosystem-cert-preflight-checks | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-ecosystem-cert-preflight-checks:0.2@sha256:1157c6ac9805af8b8874e4b8d68d2403d99e1c007f63623566b5d848b27c1826 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: sast-snyk-check | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| runAfter: | ||
| - clone-repository | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-snyk-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-snyk-check:0.4@sha256:0d22dbaa528c8edf59aafab3600a0537b5408b80a4f69dd9cb616620795ecdc8 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: clamav-scan | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: clamav-scan | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-clamav-scan:0.2@sha256:24182598bf5161c4007988a7236e240f361c77a0a9b6973a6712dbae50d296bc | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: sast-coverity-check | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: IMAGE | ||
| value: $(params.output-image) | ||
| - name: DOCKERFILE | ||
| value: $(params.dockerfile) | ||
| - name: CONTEXT | ||
| value: $(params.path-context) | ||
| - name: HERMETIC | ||
| value: $(params.hermetic) | ||
| - name: PREFETCH_INPUT | ||
| value: $(params.prefetch-input) | ||
| - name: IMAGE_EXPIRES_AFTER | ||
| value: $(params.image-expires-after) | ||
| - name: COMMIT_SHA | ||
| value: $(tasks.clone-repository.results.commit) | ||
| - name: BUILD_ARGS | ||
| value: | ||
| - $(params.build-args[*]) | ||
| - name: BUILD_ARGS_FILE | ||
| value: $(params.build-args-file) | ||
| runAfter: | ||
| - coverity-availability-check | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-coverity-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-coverity-check:0.3@sha256:d7d9682b679c98ffe4633dd24b62cafb0799264d459cf2ab9fa1593e1a42bfdf | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - input: $(tasks.coverity-availability-check.results.STATUS) | ||
| operator: in | ||
| values: | ||
| - success | ||
| workspaces: | ||
| - name: source | ||
| workspace: workspace | ||
| - name: coverity-availability-check | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: coverity-availability-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-coverity-availability-check:0.2@sha256:8b58c4fae00c0dfe3937abfb8a9a61aa3c408cca4278b817db53d518428d944e | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| - name: sast-shell-check | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-shell-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-shell-check:0.1@sha256:188a4f6a582ac43d4de46c3998ded3c2a8ee237fb0604d90559a3b6e0aa62b0f | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: sast-unicode-check | ||
| params: | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: sast-unicode-check | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-sast-unicode-check:0.2@sha256:e4a5215b45b1886a185a9db8ab392f8440c2b0848f76d719885637cf8d2628ed | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| when: | ||
| - input: $(params.skip-checks) | ||
| operator: in | ||
| values: | ||
| - "false" | ||
| workspaces: | ||
| - name: workspace | ||
| workspace: workspace | ||
| - name: apply-tags | ||
| params: | ||
| - name: IMAGE | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: apply-tags | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-apply-tags:0.1@sha256:3f89ba89cacf8547261b5ce064acce81bfe470c8ace127794d0e90aebc8c347d | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| - name: rpms-signature-scan | ||
| params: | ||
| - name: image-digest | ||
| value: $(tasks.build-container.results.IMAGE_DIGEST) | ||
| - name: image-url | ||
| value: $(tasks.build-container.results.IMAGE_URL) | ||
| runAfter: | ||
| - build-container | ||
| taskRef: | ||
| params: | ||
| - name: name | ||
| value: rpms-signature-scan | ||
| - name: bundle | ||
| value: quay.io/konflux-ci/tekton-catalog/task-rpms-signature-scan:0.2@sha256:297c2d8928aa3b114fcb1ba5d9da8b10226b68fed30706e78a6a5089c6cd30e3 | ||
| - name: kind | ||
| value: task | ||
| resolver: bundles | ||
| workspaces: | ||
| - name: workspace |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate tasks into reusable Pipeline
The inline tasks section is extensive and duplicated in the push pipeline. Extract the common pipelineSpec into a standalone Pipeline resource and reference it via pipelineRef in each PipelineRun to reduce duplication and simplify maintenance.
🤖 Prompt for AI Agents
In
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml
from lines 140 to 536, the tasks section is duplicated and extensive. To fix
this, extract the common tasks into a separate Pipeline resource defined with a
pipelineSpec containing these tasks. Then, update the PipelineRun definitions to
reference this Pipeline using pipelineRef instead of embedding the tasks inline.
This consolidation reduces duplication and simplifies maintenance.
13dc023 to
09e453b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
apps/assisted-installer-app/src/i18n.ts (3)
28-28: Consider disabling debug mode in productionThe debug mode is still hardcoded to
true, which will output verbose logs in production environments.
53-53: Replaceinstanceof ArraywithArray.isArray()The static analysis tool correctly identifies that
instanceof Arraycan fail with arrays from different execution contexts.🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
52-61: Add handler for non-arraylngparameterThe missing key handler only processes array inputs but doesn't handle string inputs, potentially missing warnings for single-language scenarios.
🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
apps/assisted-installer-app/pr_check.sh (1)
10-11: Initialize APP_ROOT before defaulting WORKSPACE
WORKSPACE=${WORKSPACE:-$APP_ROOT}runs beforeAPP_ROOTis set, so WORKSPACE may default to empty. Swap these assignments soAPP_ROOT=$(pwd)comes before the WORKSPACE default.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/build_deploy.sh (1)
10-11: Initialize APP_ROOT before defaulting WORKSPACE
WORKSPACE=${WORKSPACE:-$APP_ROOT}appears beforeAPP_ROOTis defined, leading to an empty default. MoveAPP_ROOT=$(pwd)above the WORKSPACE export.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml (1)
140-536: Consolidate inline tasks into a reusable Pipeline
Thetaskssection is large and duplicated between pull-request and push PipelineRuns. Extract the commonpipelineSpec.tasksinto a standalonePipelineresource and reference it viapipelineRefto reduce duplication and simplify maintenance.
🧹 Nitpick comments (8)
apps/assisted-installer-app/src/i18n.ts (1)
5-11: Consider making date formatting locale-awareThe
dateTimeFormatteruses'default'locale, which may not align with the user's selected language. Consider using the current i18n language for consistency with number formatting.-const dateTimeFormatter = new Intl.DateTimeFormat('default', { +const getDateTimeFormatter = (locale: string) => new Intl.DateTimeFormat(locale || 'default', { month: 'short', day: 'numeric', hour: 'numeric', minute: 'numeric', year: 'numeric', });Then update the interpolation function:
if (value instanceof Date) { - output = dateTimeFormatter.format(value); + output = getDateTimeFormatter(lng).format(value); }apps/assisted-installer-app/pr_check.sh (3)
11-11: Avoid masking return values in export
ShellCheck SC2155 warns about exporting and assigning in one statement. Split it into separate assignment and export for clarity:- export APP_ROOT=$(pwd) + APP_ROOT=$(pwd) + export APP_ROOT🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Add ShellCheck directive for dynamic source
ShellCheck SC1090 can't follow non-constant source. Suppress or document this with a directive:# shellcheck source=/dev/null source <(curl -sSL "$COMMON_BUILDER"/src/frontend-build.sh)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-22: Quote variable expansions for safety
Wrap$WORKSPACEin quotes to handle paths with spaces or special characters:- mkdir -p $WORKSPACE/artifacts - cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml + mkdir -p "$WORKSPACE/artifacts" + cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"apps/assisted-installer-app/build_deploy.sh (3)
11-12: Split assignments to avoid masking return values
ShellCheck SC2155 flags combining export with command substitution. Separate them:- export APP_ROOT=$(pwd) + APP_ROOT=$(pwd) + export APP_ROOT - export IMAGE_TAG=$(git rev-parse HEAD) + IMAGE_TAG=$(git rev-parse HEAD) + export IMAGE_TAG🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Add ShellCheck directive for dynamic source
Add a directive to acknowledge the external script source:# shellcheck source=/dev/null source <(curl -sSL "$COMMON_BUILDER"/src/frontend-build.sh)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-22: Quote variables in file operations
Ensure paths are quoted when creating directories and writing files:- mkdir -p $WORKSPACE/artifacts - cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml + mkdir -p "$WORKSPACE/artifacts" + cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"docs/DEVELOPMENT.md (1)
56-60: Clarify "package manager shims" grammar
The phrase "package managers shims" could be confusing. Consider:- 1. Set up the package managers shims and install the project dependencies: + 1. Install package manager shims and the project dependencies:🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
.gitignore(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml(1 hunks)apps/assisted-installer-app/build_deploy.sh(1 hunks)apps/assisted-installer-app/config/empty.js(1 hunks)apps/assisted-installer-app/config/jest.setup.js(1 hunks)apps/assisted-installer-app/deploy/frontend.yaml(1 hunks)apps/assisted-installer-app/fec.config.js(1 hunks)apps/assisted-installer-app/jest.config.js(1 hunks)apps/assisted-installer-app/package.json(1 hunks)apps/assisted-installer-app/pr_check.sh(1 hunks)apps/assisted-installer-app/src/bootstrap.ts(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/ClusterStatus.tsx(1 hunks)apps/assisted-installer-app/src/components/ExternalLink.tsx(1 hunks)apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx(1 hunks)apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx(1 hunks)apps/assisted-installer-app/src/components/NoPermissionsError.tsx(1 hunks)apps/assisted-installer-app/src/components/RootApp.tsx(1 hunks)apps/assisted-installer-app/src/components/Services.tsx(1 hunks)apps/assisted-installer-app/src/components/TechnologyPreview.tsx(1 hunks)apps/assisted-installer-app/src/entry.ts(1 hunks)apps/assisted-installer-app/src/hooks/useInitApp.ts(1 hunks)apps/assisted-installer-app/src/i18n.ts(1 hunks)apps/assisted-installer-app/src/index.html(1 hunks)apps/assisted-installer-app/tsconfig.json(1 hunks)docs/DEVELOPMENT.md(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (24)
- .gitignore
- apps/assisted-installer-app/config/jest.setup.js
- apps/assisted-installer-app/config/empty.js
- apps/assisted-installer-app/tsconfig.json
- apps/assisted-installer-app/src/components/TechnologyPreview.tsx
- apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx
- apps/assisted-installer-app/src/components/NoPermissionsError.tsx
- apps/assisted-installer-app/src/bootstrap.ts
- apps/assisted-installer-app/src/components/ClusterStatus.tsx
- apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
- apps/assisted-installer-app/src/components/ExternalLink.tsx
- package.json
- apps/assisted-installer-app/src/index.html
- apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx
- apps/assisted-installer-app/src/hooks/useInitApp.ts
- apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx
- apps/assisted-installer-app/src/entry.ts
- apps/assisted-installer-app/deploy/frontend.yaml
- apps/assisted-installer-app/jest.config.js
- apps/assisted-installer-app/src/components/RootApp.tsx
- apps/assisted-installer-app/src/components/Services.tsx
- apps/assisted-installer-app/fec.config.js
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
- apps/assisted-installer-app/package.json
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/assisted-installer-app/build_deploy.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/pr_check.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🪛 Biome (1.9.4)
apps/assisted-installer-app/src/i18n.ts
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🪛 LanguageTool
docs/DEVELOPMENT.md
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
apps/assisted-installer-app/src/i18n.ts (3)
28-28: Consider disabling debug mode in productionThe debug mode is still hardcoded to
true, which will output verbose logs in production environments.
53-53: Replaceinstanceof ArraywithArray.isArray()The static analysis tool correctly identifies that
instanceof Arraycan fail with arrays from different execution contexts.🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
52-61: Add handler for non-arraylngparameterThe missing key handler only processes array inputs but doesn't handle string inputs, potentially missing warnings for single-language scenarios.
🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
apps/assisted-installer-app/pr_check.sh (1)
10-11: Initialize APP_ROOT before defaulting WORKSPACE
WORKSPACE=${WORKSPACE:-$APP_ROOT}runs beforeAPP_ROOTis set, so WORKSPACE may default to empty. Swap these assignments soAPP_ROOT=$(pwd)comes before the WORKSPACE default.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/build_deploy.sh (1)
10-11: Initialize APP_ROOT before defaulting WORKSPACE
WORKSPACE=${WORKSPACE:-$APP_ROOT}appears beforeAPP_ROOTis defined, leading to an empty default. MoveAPP_ROOT=$(pwd)above the WORKSPACE export.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml (1)
140-536: Consolidate inline tasks into a reusable Pipeline
Thetaskssection is large and duplicated between pull-request and push PipelineRuns. Extract the commonpipelineSpec.tasksinto a standalonePipelineresource and reference it viapipelineRefto reduce duplication and simplify maintenance.
🧹 Nitpick comments (8)
apps/assisted-installer-app/src/i18n.ts (1)
5-11: Consider making date formatting locale-awareThe
dateTimeFormatteruses'default'locale, which may not align with the user's selected language. Consider using the current i18n language for consistency with number formatting.-const dateTimeFormatter = new Intl.DateTimeFormat('default', { +const getDateTimeFormatter = (locale: string) => new Intl.DateTimeFormat(locale || 'default', { month: 'short', day: 'numeric', hour: 'numeric', minute: 'numeric', year: 'numeric', });Then update the interpolation function:
if (value instanceof Date) { - output = dateTimeFormatter.format(value); + output = getDateTimeFormatter(lng).format(value); }apps/assisted-installer-app/pr_check.sh (3)
11-11: Avoid masking return values in export
ShellCheck SC2155 warns about exporting and assigning in one statement. Split it into separate assignment and export for clarity:- export APP_ROOT=$(pwd) + APP_ROOT=$(pwd) + export APP_ROOT🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Add ShellCheck directive for dynamic source
ShellCheck SC1090 can't follow non-constant source. Suppress or document this with a directive:# shellcheck source=/dev/null source <(curl -sSL "$COMMON_BUILDER"/src/frontend-build.sh)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-22: Quote variable expansions for safety
Wrap$WORKSPACEin quotes to handle paths with spaces or special characters:- mkdir -p $WORKSPACE/artifacts - cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml + mkdir -p "$WORKSPACE/artifacts" + cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"apps/assisted-installer-app/build_deploy.sh (3)
11-12: Split assignments to avoid masking return values
ShellCheck SC2155 flags combining export with command substitution. Separate them:- export APP_ROOT=$(pwd) + APP_ROOT=$(pwd) + export APP_ROOT - export IMAGE_TAG=$(git rev-parse HEAD) + IMAGE_TAG=$(git rev-parse HEAD) + export IMAGE_TAG🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Add ShellCheck directive for dynamic source
Add a directive to acknowledge the external script source:# shellcheck source=/dev/null source <(curl -sSL "$COMMON_BUILDER"/src/frontend-build.sh)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-22: Quote variables in file operations
Ensure paths are quoted when creating directories and writing files:- mkdir -p $WORKSPACE/artifacts - cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml + mkdir -p "$WORKSPACE/artifacts" + cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"docs/DEVELOPMENT.md (1)
56-60: Clarify "package manager shims" grammar
The phrase "package managers shims" could be confusing. Consider:- 1. Set up the package managers shims and install the project dependencies: + 1. Install package manager shims and the project dependencies:🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
.gitignore(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml(1 hunks)apps/assisted-installer-app/build_deploy.sh(1 hunks)apps/assisted-installer-app/config/empty.js(1 hunks)apps/assisted-installer-app/config/jest.setup.js(1 hunks)apps/assisted-installer-app/deploy/frontend.yaml(1 hunks)apps/assisted-installer-app/fec.config.js(1 hunks)apps/assisted-installer-app/jest.config.js(1 hunks)apps/assisted-installer-app/package.json(1 hunks)apps/assisted-installer-app/pr_check.sh(1 hunks)apps/assisted-installer-app/src/bootstrap.ts(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/ClusterStatus.tsx(1 hunks)apps/assisted-installer-app/src/components/ExternalLink.tsx(1 hunks)apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx(1 hunks)apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx(1 hunks)apps/assisted-installer-app/src/components/NoPermissionsError.tsx(1 hunks)apps/assisted-installer-app/src/components/RootApp.tsx(1 hunks)apps/assisted-installer-app/src/components/Services.tsx(1 hunks)apps/assisted-installer-app/src/components/TechnologyPreview.tsx(1 hunks)apps/assisted-installer-app/src/entry.ts(1 hunks)apps/assisted-installer-app/src/hooks/useInitApp.ts(1 hunks)apps/assisted-installer-app/src/i18n.ts(1 hunks)apps/assisted-installer-app/src/index.html(1 hunks)apps/assisted-installer-app/tsconfig.json(1 hunks)docs/DEVELOPMENT.md(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (24)
- .gitignore
- apps/assisted-installer-app/config/jest.setup.js
- apps/assisted-installer-app/config/empty.js
- apps/assisted-installer-app/tsconfig.json
- apps/assisted-installer-app/src/components/TechnologyPreview.tsx
- apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx
- apps/assisted-installer-app/src/components/NoPermissionsError.tsx
- apps/assisted-installer-app/src/bootstrap.ts
- apps/assisted-installer-app/src/components/ClusterStatus.tsx
- apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
- apps/assisted-installer-app/src/components/ExternalLink.tsx
- package.json
- apps/assisted-installer-app/src/index.html
- apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx
- apps/assisted-installer-app/src/hooks/useInitApp.ts
- apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx
- apps/assisted-installer-app/src/entry.ts
- apps/assisted-installer-app/deploy/frontend.yaml
- apps/assisted-installer-app/jest.config.js
- apps/assisted-installer-app/src/components/RootApp.tsx
- apps/assisted-installer-app/src/components/Services.tsx
- apps/assisted-installer-app/fec.config.js
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
- apps/assisted-installer-app/package.json
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/assisted-installer-app/build_deploy.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/pr_check.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🪛 Biome (1.9.4)
apps/assisted-installer-app/src/i18n.ts
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🪛 LanguageTool
docs/DEVELOPMENT.md
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
rawagner
force-pushed
the
installer_app_monorepo
branch
from
09e453b to
9dc38f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/assisted-installer-app/src/i18n.ts (2)
28-28: Consider disabling debug mode in productionDebug mode should be conditionally enabled only during development to avoid verbose console logging in production environments.
53-60: Replaceinstanceof ArraywithArray.isArray()and handle string parameterThe missing key handler has two issues that have been previously identified:
- Using
instanceof Arrayinstead of the more reliableArray.isArray()- Only handling array inputs, ignoring when
lngis a string🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
apps/assisted-installer-app/pr_check.sh (1)
10-11: InitializeAPP_ROOTbefore defaultingWORKSPACE
WORKSPACE=${WORKSPACE:-$APP_ROOT}usesAPP_ROOTbefore it's set, causingWORKSPACEto default to empty. Swap these assignments soAPP_ROOTis defined first.-export WORKSPACE=${WORKSPACE:-$APP_ROOT} -export APP_ROOT=$(pwd) +APP_ROOT=$(pwd) +export APP_ROOT +export WORKSPACE=${WORKSPACE:-$APP_ROOT}🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/build_deploy.sh (1)
10-11: InitializeAPP_ROOTbefore defaultingWORKSPACE
As inpr_check.sh,WORKSPACE=${WORKSPACE:-$APP_ROOT}appears beforeAPP_ROOTis defined. Move theAPP_ROOTassignment above theWORKSPACEexport:-export WORKSPACE=${WORKSPACE:-$APP_ROOT} -export APP_ROOT=$(pwd) +APP_ROOT=$(pwd) +export APP_ROOT +export WORKSPACE=${WORKSPACE:-$APP_ROOT}🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
🧹 Nitpick comments (6)
apps/assisted-installer-app/pr_check.sh (2)
11-11: Separate declaration and export to preserve exit codes
Combining assignment and export (export APP_ROOT=$(pwd)) masks the exit status ofpwd. Split into two steps:-export APP_ROOT=$(pwd) +APP_ROOT=$(pwd) +export APP_ROOT🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Handle dynamic sourcing and improve stability
ShellCheck (SC1090) can’t trace non-constantsource. Add a disable directive and consider pinning the remote script to a specific commit/tag to avoid unexpected changes:-# source is preferred to | bash -s in this case to avoid a subshell -source <(curl -sSL $COMMON_BUILDER/src/frontend-build.sh) +# shellcheck disable=SC1090 +# source is preferred to | bash -s in this case to avoid a subshell +source <(curl -sSL $COMMON_BUILDER/src/frontend-build.sh@<commit-hash>)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/build_deploy.sh (2)
12-12: Separate declaration and export forIMAGE_TAG
Combiningexport IMAGE_TAG=$(git rev-parse HEAD)masks the exit status ofgit. Split into:-export IMAGE_TAG=$(git rev-parse HEAD) +IMAGE_TAG=$(git rev-parse HEAD) +export IMAGE_TAG🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Handle dynamic sourcing and pin remote builder
ShellCheck (SC1090) warns about sourcing non-constant content. Add a disable directive and pin the URL to a stable ref/tag:-# source is preferred to | bash -s in this case to avoid a subshell -source <(curl -sSL $COMMON_BUILDER/src/frontend-build.sh) +# shellcheck disable=SC1090 +# source is preferred to | bash -s in this case to avoid a subshell +source <(curl -sSL $COMMON_BUILDER/src/frontend-build.sh@<commit-hash>)🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
docs/DEVELOPMENT.md (2)
56-56: Refine grammar for clarity
“package managers shims” reads awkwardly. Replace with “package manager shims” for correct noun agreement:-1. Set up the package managers shims and install the project dependencies: +1. Set up the package manager shims and install the project dependencies:🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
62-65: Clarify script naming consistency
The heading uses hyphens (assisted-installer-app), but the npm script uses underscores. Enclose script names in backticks and align terminology:-2. Start the assisted-installer-app project: +2. Start the `assisted_installer_app` project:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
.gitignore(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml(1 hunks)apps/assisted-installer-app/build_deploy.sh(1 hunks)apps/assisted-installer-app/config/empty.js(1 hunks)apps/assisted-installer-app/config/jest.setup.js(1 hunks)apps/assisted-installer-app/deploy/frontend.yaml(1 hunks)apps/assisted-installer-app/fec.config.js(1 hunks)apps/assisted-installer-app/jest.config.js(1 hunks)apps/assisted-installer-app/package.json(1 hunks)apps/assisted-installer-app/pr_check.sh(1 hunks)apps/assisted-installer-app/src/bootstrap.ts(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/ClusterStatus.tsx(1 hunks)apps/assisted-installer-app/src/components/ExternalLink.tsx(1 hunks)apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx(1 hunks)apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx(1 hunks)apps/assisted-installer-app/src/components/NoPermissionsError.tsx(1 hunks)apps/assisted-installer-app/src/components/RootApp.tsx(1 hunks)apps/assisted-installer-app/src/components/Services.tsx(1 hunks)apps/assisted-installer-app/src/components/TechnologyPreview.tsx(1 hunks)apps/assisted-installer-app/src/entry.ts(1 hunks)apps/assisted-installer-app/src/hooks/useInitApp.ts(1 hunks)apps/assisted-installer-app/src/i18n.ts(1 hunks)apps/assisted-installer-app/src/index.html(1 hunks)apps/assisted-installer-app/tsconfig.json(1 hunks)docs/DEVELOPMENT.md(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (25)
- apps/assisted-installer-app/config/jest.setup.js
- apps/assisted-installer-app/config/empty.js
- .gitignore
- apps/assisted-installer-app/src/components/Services.tsx
- apps/assisted-installer-app/src/bootstrap.ts
- apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx
- apps/assisted-installer-app/src/index.html
- apps/assisted-installer-app/src/components/ExternalLink.tsx
- apps/assisted-installer-app/tsconfig.json
- apps/assisted-installer-app/src/components/RootApp.tsx
- apps/assisted-installer-app/src/entry.ts
- apps/assisted-installer-app/jest.config.js
- apps/assisted-installer-app/src/components/NoPermissionsError.tsx
- apps/assisted-installer-app/package.json
- apps/assisted-installer-app/deploy/frontend.yaml
- apps/assisted-installer-app/fec.config.js
- apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx
- package.json
- apps/assisted-installer-app/src/components/ClusterStatus.tsx
- apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
- apps/assisted-installer-app/src/components/TechnologyPreview.tsx
- apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx
- apps/assisted-installer-app/src/hooks/useInitApp.ts
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/assisted-installer-app/build_deploy.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/pr_check.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🪛 Biome (1.9.4)
apps/assisted-installer-app/src/i18n.ts
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🪛 LanguageTool
docs/DEVELOPMENT.md
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🔇 Additional comments (4)
apps/assisted-installer-app/src/i18n.ts (4)
1-3: LGTM: Clean imports and setupThe imports are properly structured for i18next with React integration and the translation resource is correctly imported from the shared locales package.
5-11: LGTM: Well-configured date formatterThe date formatter setup provides consistent date/time formatting across the application with appropriate locale options.
13-27: LGTM: Solid i18n configurationThe i18n configuration is well-structured with appropriate settings for:
- English-only support with proper fallbacks
- Custom namespace separator to avoid conflicts
- Disabled key separator for flexibility
- React-specific optimizations with suspense and HTML support
- Proper interpolation escaping for React
Also applies to: 29-51, 61-62
30-46: LGTM: Robust interpolation formattingThe custom format function properly handles number and date formatting using modern Intl APIs, providing consistent localized output across different data types.
rawagner
force-pushed
the
installer_app_monorepo
branch
from
9dc38f3 to
07e515a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
apps/assisted-installer-app/pr_check.sh (1)
10-11: Initialize APP_ROOT before defaulting WORKSPACE
The script setsWORKSPACE=${WORKSPACE:-$APP_ROOT}(line 10) beforeAPP_ROOT=$(pwd)is defined (line 11), causingWORKSPACEto default to an empty value. Swap these two lines so thatAPP_ROOTis established before it's used as the default.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/build_deploy.sh (1)
10-11: Initialize APP_ROOT before defaulting WORKSPACE
The script defaultsWORKSPACE=${WORKSPACE:-$APP_ROOT}(line 10) before definingAPP_ROOT(line 11), potentially resulting in an emptyWORKSPACE. Move theAPP_ROOT=$(pwd)assignment above theWORKSPACEexport.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/src/i18n.ts (3)
28-28: Consider disabling debug mode in production.The debug mode setting should be conditional to prevent verbose logging in production environments.
53-53: Replace instanceof Array with Array.isArray().Using
instanceof Arraycan return false for arrays from different execution contexts. This issue was previously identified.🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
52-61: Add handler for non-arraylngparameter.The missing key handler only processes array inputs but doesn't handle string inputs. This issue was previously identified.
🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
🧹 Nitpick comments (7)
apps/assisted-installer-app/pr_check.sh (2)
17-17: Suppress ShellCheck SC1090 or vendor the builder script
ShellCheck flagssource <(curl ...)as SC1090 since it can’t follow non-constant sources. You can add a directive above to disable SC1090:# shellcheck disable=SC1090Or consider vendoring
frontend-build.shlocally to avoid dynamic sourcing.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-26: Quote variable expansions to handle paths with spaces
Wrap uses of$WORKSPACEin quotes to guard against directory names with spaces:-mkdir -p $WORKSPACE/artifacts +mkdir -p "$WORKSPACE/artifacts" -cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml +cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"apps/assisted-installer-app/build_deploy.sh (3)
12-12: Separate assignment and export for IMAGE_TAG
To avoid masking return values and improve clarity, split the command substitution from the export:-export IMAGE_TAG=$(git rev-parse HEAD) +IMAGE_TAG=$(git rev-parse HEAD) +export IMAGE_TAG🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
17-17: Suppress ShellCheck SC1090 or vendor the builder script
As inpr_check.sh, ShellCheck will warn on dynamic sourcing. Add:# shellcheck disable=SC1090above line 17, or vendor the remote script locally.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-26: Quote variable expansions to handle paths with spaces
Apply quoting to$WORKSPACEwhen creating artifact directories and files:-mkdir -p $WORKSPACE/artifacts +mkdir -p "$WORKSPACE/artifacts" -cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml +cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"docs/DEVELOPMENT.md (2)
69-73: Provide an HTTPS cloning alternative
Cloning via SSH requires key access. For broader accessibility, you might also document:git clone https://github.com/RedHatInsights/uhc-portal.git
75-80: Specify code-fence language for syntax highlighting
Add thebashlanguage identifier to code fences:- ``` + ```bash yarn start --env ai_standalone<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [style] ~77-~77: Consider a more concise word here. Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas... (IN_ORDER_TO_PREMIUM) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 9dc38f3d1f4b6a1a649ec27411f2798502467b4e and 07e515a0f01eab734e685f98075b5abac8cef0ed. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `yarn.lock` is excluded by `!**/yarn.lock`, `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (29)</summary> * `.gitignore` (1 hunks) * `apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml` (1 hunks) * `apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml` (1 hunks) * `apps/assisted-installer-app/build_deploy.sh` (1 hunks) * `apps/assisted-installer-app/config/empty.js` (1 hunks) * `apps/assisted-installer-app/config/jest.setup.js` (1 hunks) * `apps/assisted-installer-app/deploy/frontend.yaml` (1 hunks) * `apps/assisted-installer-app/fec.config.js` (1 hunks) * `apps/assisted-installer-app/jest.config.js` (1 hunks) * `apps/assisted-installer-app/package.json` (1 hunks) * `apps/assisted-installer-app/pr_check.sh` (1 hunks) * `apps/assisted-installer-app/src/bootstrap.ts` (1 hunks) * `apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/ClusterStatus.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/ExternalLink.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/NoPermissionsError.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/RootApp.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/Services.tsx` (1 hunks) * `apps/assisted-installer-app/src/components/TechnologyPreview.tsx` (1 hunks) * `apps/assisted-installer-app/src/entry.ts` (1 hunks) * `apps/assisted-installer-app/src/hooks/useInitApp.ts` (1 hunks) * `apps/assisted-installer-app/src/i18n.ts` (1 hunks) * `apps/assisted-installer-app/src/index.html` (1 hunks) * `apps/assisted-installer-app/tsconfig.json` (1 hunks) * `docs/DEVELOPMENT.md` (1 hunks) * `package.json` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (25)</summary> * apps/assisted-installer-app/src/components/Services.tsx * apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx * apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx * apps/assisted-installer-app/src/components/ExternalLink.tsx * apps/assisted-installer-app/config/empty.js * apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx * apps/assisted-installer-app/src/components/ClusterStatus.tsx * apps/assisted-installer-app/config/jest.setup.js * apps/assisted-installer-app/src/index.html * .gitignore * apps/assisted-installer-app/jest.config.js * apps/assisted-installer-app/src/entry.ts * apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx * apps/assisted-installer-app/tsconfig.json * package.json * apps/assisted-installer-app/src/bootstrap.ts * apps/assisted-installer-app/src/components/TechnologyPreview.tsx * apps/assisted-installer-app/src/components/NoPermissionsError.tsx * apps/assisted-installer-app/src/components/RootApp.tsx * apps/assisted-installer-app/src/hooks/useInitApp.ts * apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml * apps/assisted-installer-app/package.json * apps/assisted-installer-app/fec.config.js * apps/assisted-installer-app/deploy/frontend.yaml * apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 Shellcheck (0.10.0)</summary> <details> <summary>apps/assisted-installer-app/build_deploy.sh</summary> [warning] 11-11: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 12-12: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location. (SC1090) </details> <details> <summary>apps/assisted-installer-app/pr_check.sh</summary> [warning] 11-11: Declare and assign separately to avoid masking return values. (SC2155) --- [warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location. (SC1090) </details> </details> <details> <summary>🪛 Biome (1.9.4)</summary> <details> <summary>apps/assisted-installer-app/src/i18n.ts</summary> [error] 53-53: Use Array.isArray() instead of instanceof Array. instanceof Array returns false for array-like objects and arrays from other execution contexts. Unsafe fix: Use Array.isArray() instead. (lint/suspicious/useIsArray) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/DEVELOPMENT.md</summary> [uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it. Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc... (AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (6)</summary> * GitHub Check: tests * GitHub Check: lint * GitHub Check: translation-files * GitHub Check: unit-tests * GitHub Check: circular-deps * GitHub Check: format </details> <details> <summary>🔇 Additional comments (7)</summary><blockquote> <details> <summary>docs/DEVELOPMENT.md (2)</summary> `56-60`: **Setup dependencies step is accurate** The instructions for installing dependencies via `yarn install` are clear and correct. No action required here. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it. Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc... (AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER) </details> </details> --- `62-67`: **Verify that the `start` script name matches `package.json`** The docs refer to `yarn start:assisted_installer_app`. Please confirm that this exact key exists in the root `package.json` (underscores vs hyphens). Update either the script name or the docs for consistency. </details> <details> <summary>apps/assisted-installer-app/src/i18n.ts (5)</summary> `1-3`: **LGTM! Clean imports setup.** The imports are well-structured and follow standard i18next patterns. --- `13-27`: **LGTM! Solid i18n configuration.** The configuration is well-structured with appropriate settings for English-only support and proper fallback handling. --- `30-46`: **LGTM! Well-implemented interpolation formatting.** The custom interpolation function properly handles both numbers and dates with appropriate formatters. --- `48-51`: **LGTM! Good React-specific configuration.** The React settings are appropriate, enabling suspense and basic HTML support in translations. --- `64-64`: **LGTM! Clean export.** The default export is appropriate for the i18n instance. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| const dateTimeFormatter = new Intl.DateTimeFormat('default', { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| hour: 'numeric', | ||
| minute: 'numeric', | ||
| year: 'numeric', | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix locale inconsistency in date formatter.
The date formatter uses 'default' locale, which doesn't respect the current language setting. This creates inconsistency with the number formatter that correctly uses the lng parameter.
Move the date formatter creation inside the interpolation function to respect the current language:
-const dateTimeFormatter = new Intl.DateTimeFormat('default', {
- month: 'short',
- day: 'numeric',
- hour: 'numeric',
- minute: 'numeric',
- year: 'numeric',
-});Then update the interpolation function:
if (value instanceof Date) {
- output = dateTimeFormatter.format(value);
+ const dateTimeFormatter = new Intl.DateTimeFormat(lng, {
+ month: 'short',
+ day: 'numeric',
+ hour: 'numeric',
+ minute: 'numeric',
+ year: 'numeric',
+ });
+ output = dateTimeFormatter.format(value);
}📝 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.
| const dateTimeFormatter = new Intl.DateTimeFormat('default', { | |
| month: 'short', | |
| day: 'numeric', | |
| hour: 'numeric', | |
| minute: 'numeric', | |
| year: 'numeric', | |
| }); | |
| -const dateTimeFormatter = new Intl.DateTimeFormat('default', { | |
| - month: 'short', | |
| - day: 'numeric', | |
| - hour: 'numeric', | |
| - minute: 'numeric', | |
| - year: 'numeric', | |
| -}); | |
| // … other code … | |
| if (value instanceof Date) { | |
| - output = dateTimeFormatter.format(value); | |
| + const dateTimeFormatter = new Intl.DateTimeFormat(lng, { | |
| + month: 'short', | |
| + day: 'numeric', | |
| + hour: 'numeric', | |
| + minute: 'numeric', | |
| + year: 'numeric', | |
| + }); | |
| + output = dateTimeFormatter.format(value); | |
| } |
🤖 Prompt for AI Agents
In apps/assisted-installer-app/src/i18n.ts around lines 5 to 11, the date
formatter uses a fixed 'default' locale, causing inconsistency with the number
formatter that uses the dynamic `lng` parameter. To fix this, move the creation
of the dateTimeFormatter inside the interpolation function so it can use the
current `lng` value as the locale. Update the interpolation function to
instantiate the date formatter with the `lng` parameter each time it runs,
ensuring the date formatting respects the current language setting.
07e515a to
8b62d9f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
apps/assisted-installer-app/src/i18n.ts (4)
28-28: Consider disabling debug mode in productionThe i18n configuration has
debug: true, which will output verbose logs to the console. This is useful during development but should be disabled in production.
53-53: Replaceinstanceof ArraywithArray.isArray()The static analysis tool correctly flagged that
instanceof Arraycan lead to false negatives with arrays from different execution contexts (e.g., iframes). Use the more reliableArray.isArray()method instead.🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
52-61: Add handler for non-arraylngparameterThe missing key handler only processes array inputs but doesn't handle the case where
lngmight be a string. This could lead to missing warnings for single-language missing key scenarios.🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
5-11: Fix locale inconsistency in date formatterThe date formatter uses 'default' locale, which doesn't respect the current language setting. This creates inconsistency with the number formatter that correctly uses the
lngparameter.apps/assisted-installer-app/pr_check.sh (1)
10-11: InitializeAPP_ROOTbefore defaultingWORKSPACE.
WORKSPACE=${WORKSPACE:-$APP_ROOT}runs beforeAPP_ROOTis defined, causingWORKSPACEto default to empty. Move theexport APP_ROOT=$(pwd)line above theexport WORKSPACEline.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/build_deploy.sh (1)
10-13: InitializeAPP_ROOTbefore defaultingWORKSPACE.
Theexport WORKSPACE=${WORKSPACE:-$APP_ROOT}line precedes the definition ofAPP_ROOT, soWORKSPACEwill always be empty. Moveexport APP_ROOT=$(pwd)above theexport WORKSPACEline.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
🧹 Nitpick comments (2)
apps/assisted-installer-app/pr_check.sh (1)
17-17: Add ShellCheck directive for non-constant source.
ShellCheck (SC1090) cannot follow a dynamic source. Suppress or clarify this by adding, for example:# shellcheck disable=SC1090 source <(curl -sSL "$COMMON_BUILDER/src/frontend-build.sh")🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/build_deploy.sh (1)
17-17: Add ShellCheck directive for non-constant source.
To satisfy ShellCheck (SC1090) when sourcing a remote script, add a directive such as:# shellcheck disable=SC1090above the
source <(curl -sSL "$COMMON_BUILDER/src/frontend-build.sh")line.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (28)
.eslintrc.json(1 hunks).gitignore(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml(1 hunks)apps/assisted-installer-app/build_deploy.sh(1 hunks)apps/assisted-installer-app/deploy/frontend.yaml(1 hunks)apps/assisted-installer-app/fec.config.js(1 hunks)apps/assisted-installer-app/jest.config.js(1 hunks)apps/assisted-installer-app/package.json(1 hunks)apps/assisted-installer-app/pr_check.sh(1 hunks)apps/assisted-installer-app/src/bootstrap.ts(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/ClusterStatus.tsx(1 hunks)apps/assisted-installer-app/src/components/ExternalLink.tsx(1 hunks)apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx(1 hunks)apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx(1 hunks)apps/assisted-installer-app/src/components/NoPermissionsError.tsx(1 hunks)apps/assisted-installer-app/src/components/RootApp.tsx(1 hunks)apps/assisted-installer-app/src/components/Services.tsx(1 hunks)apps/assisted-installer-app/src/components/TechnologyPreview.tsx(1 hunks)apps/assisted-installer-app/src/entry.ts(1 hunks)apps/assisted-installer-app/src/hooks/useInitApp.ts(1 hunks)apps/assisted-installer-app/src/i18n.ts(1 hunks)apps/assisted-installer-app/src/index.html(1 hunks)apps/assisted-installer-app/tsconfig.json(1 hunks)docs/DEVELOPMENT.md(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .eslintrc.json
🚧 Files skipped from review as they are similar to previous changes (23)
- .gitignore
- apps/assisted-installer-app/src/entry.ts
- package.json
- apps/assisted-installer-app/src/components/RootApp.tsx
- apps/assisted-installer-app/src/components/Services.tsx
- apps/assisted-installer-app/src/components/ClusterStatus.tsx
- apps/assisted-installer-app/src/bootstrap.ts
- apps/assisted-installer-app/tsconfig.json
- apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx
- apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
- apps/assisted-installer-app/src/components/NoPermissionsError.tsx
- apps/assisted-installer-app/src/components/ExternalLink.tsx
- apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx
- apps/assisted-installer-app/src/index.html
- apps/assisted-installer-app/src/components/TechnologyPreview.tsx
- apps/assisted-installer-app/src/hooks/useInitApp.ts
- apps/assisted-installer-app/jest.config.js
- apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx
- apps/assisted-installer-app/deploy/frontend.yaml
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
- apps/assisted-installer-app/package.json
- apps/assisted-installer-app/fec.config.js
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml
🧰 Additional context used
🪛 LanguageTool
docs/DEVELOPMENT.md
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🪛 Shellcheck (0.10.0)
apps/assisted-installer-app/build_deploy.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/pr_check.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🪛 Biome (1.9.4)
apps/assisted-installer-app/src/i18n.ts
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tests
🔇 Additional comments (1)
docs/DEVELOPMENT.md (1)
56-80: Documentation for OCM integration is accurate.
The revised instructions for installing dependencies and startingassisted-installer-app(yarn start:assisted_installer_app) correctly reflect the new monorepo structure and align with the added build scripts.🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...OCM (uhc-portal) 1. Set up the package managers shims and install the project dependenc...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~77-~77: Consider a more concise word here.
Context: ...ob/master/README.md#running-locally) in order to set up their dev-environment. ```bas...(IN_ORDER_TO_PREMIUM)
rawagner
force-pushed
the
installer_app_monorepo
branch
2 times, most recently
from
5666a6f to
b8b36f8
Compare
rawagner
force-pushed
the
installer_app_monorepo
branch
from
b8b36f8 to
7d9b0be
Compare
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. 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. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
apps/assisted-installer-app/src/i18n.ts (4)
5-11: Use current locale when formatting dates (repeat feedback)
dateTimeFormatteris created with the fixed'default'locale, so dates ignore the active language (lng). Move the formatter inside the interpolation branch and passlngto keep date/number behaviour consistent with the number formatter.-const dateTimeFormatter = new Intl.DateTimeFormat('default', { - month: 'short', - day: 'numeric', - hour: 'numeric', - minute: 'numeric', - year: 'numeric', -}); ... - if (value instanceof Date) { - output = dateTimeFormatter.format(value); - } + if (value instanceof Date) { + output = new Intl.DateTimeFormat(lng, { + month: 'short', + day: 'numeric', + hour: 'numeric', + minute: 'numeric', + year: 'numeric', + }).format(value); + }Also applies to: 40-42
28-28: Disable verbose i18n debug logs in production (repeat feedback)Leaving
debug: truehard-coded will spam the console in production builds.- debug: true, + debug: process.env.NODE_ENV !== 'production',
53-53: PreferArray.isArray()overinstanceof Array(repeat feedback)
instanceof Arrayfails across realms (e.g. iframes). Use the realm-safe helper instead.- if (lng instanceof Array) { + if (Array.isArray(lng)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
52-61: Handle single-languagelnginmissingKeyHandler(repeat feedback)When
lngis a string the handler silently ignores missing keys. Log the warning for that case too.- } + } else if (typeof lng === 'string') { + // eslint-disable-next-line no-console + console.warn( + `Missing i18n key '${key}' in namespace '${ns}' and language '${lng}.'`, + ); + }🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
apps/assisted-installer-app/pr_check.sh (1)
10-11:⚠️ Potential issueEnsure APP_ROOT is defined before using it for WORKSPACE default
CurrentlyWORKSPACE=${WORKSPACE:-$APP_ROOT}is set beforeAPP_ROOTis initialized, causingWORKSPACEto default to empty. Swap the assignments so thatAPP_ROOT=$(pwd)comes before theWORKSPACEdefault.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
apps/assisted-installer-app/build_deploy.sh (1)
10-11:⚠️ Potential issueEnsure APP_ROOT is defined before using it for WORKSPACE default
The exportWORKSPACE=${WORKSPACE:-$APP_ROOT}precedes the assignmentAPP_ROOT=$(pwd), leading to an empty default. Swap these lines.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
🧹 Nitpick comments (8)
apps/assisted-installer-app/pr_check.sh (4)
11-12: Split assignment and export to preserve exit codes
Usingexport APP_ROOT=$(pwd)masks the exit code ofpwd. Split into two steps and address the ordering issue in one go:-export WORKSPACE=${WORKSPACE:-$APP_ROOT} -export APP_ROOT=$(pwd) +APP_ROOT=$(pwd) +export APP_ROOT +WORKSPACE=${WORKSPACE:-$APP_ROOT} +export WORKSPACE🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
15-17: Harden error handling and silence ShellCheck SC1090
- Add
set -o pipefailto catch pipeline failures:set -euxo pipefail- Suppress SC1090 for sourcing a non-constant URL:
# shellcheck disable=SC1090- Consider pinning
COMMON_BUILDERto a specific commit instead ofmasterto avoid upstream breakages.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-26: Quote variables when creating artifacts
Unquoted variables can break if paths contain spaces. Use:-mkdir -p $WORKSPACE/artifacts -cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml +mkdir -p "$WORKSPACE/artifacts" +cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"
29-29: Quote exit status
Wrap the exit code in quotes to prevent unintended word splitting:-exit $BUILD_RESULTS +exit "$BUILD_RESULTS"apps/assisted-installer-app/build_deploy.sh (4)
11-13: Split assignments and exports to preserve exit codes
Assignments likeexport APP_ROOT=$(pwd)andexport IMAGE_TAG=$(git rev-parse HEAD)mask failure codes. Refactor:- export APP_ROOT=$(pwd) - export IMAGE_TAG=$(git rev-parse HEAD) + APP_ROOT=$(pwd) + export APP_ROOT + IMAGE_TAG=$(git rev-parse HEAD) + export IMAGE_TAGThis ensures errors in
pwdorgit rev-parsearen’t masked.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
15-17: Improve error handling and ShellCheck compliance
- Add
set -o pipefailalongside existing flags:set -euxo pipefail- Suppress SC1090 for dynamic sourcing:
# shellcheck disable=SC1090 source <(curl -sSL "$COMMON_BUILDER"/src/frontend-build.sh)- Pin the builder script to a fixed commit SHA instead of
masterto guarantee reproducible builds.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
21-26: Quote variables when writing test artifacts
To handle any whitespace in paths, wrap variable expansions in quotes:- mkdir -p $WORKSPACE/artifacts - cat << EOF > $WORKSPACE/artifacts/junit-dummy.xml + mkdir -p "$WORKSPACE/artifacts" + cat << EOF > "$WORKSPACE/artifacts/junit-dummy.xml"
29-29: Quote exit status
Ensure the exit code is quoted:- exit $BUILD_RESULTS + exit "$BUILD_RESULTS"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (27)
.eslintrc.json(1 hunks).gitignore(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml(1 hunks)apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml(1 hunks)apps/assisted-installer-app/build_deploy.sh(1 hunks)apps/assisted-installer-app/deploy/frontend.yaml(1 hunks)apps/assisted-installer-app/fec.config.js(1 hunks)apps/assisted-installer-app/package.json(1 hunks)apps/assisted-installer-app/pr_check.sh(1 hunks)apps/assisted-installer-app/src/bootstrap.ts(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx(1 hunks)apps/assisted-installer-app/src/components/ClusterStatus.tsx(1 hunks)apps/assisted-installer-app/src/components/ExternalLink.tsx(1 hunks)apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx(1 hunks)apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx(1 hunks)apps/assisted-installer-app/src/components/NoPermissionsError.tsx(1 hunks)apps/assisted-installer-app/src/components/RootApp.tsx(1 hunks)apps/assisted-installer-app/src/components/Services.tsx(1 hunks)apps/assisted-installer-app/src/components/TechnologyPreview.tsx(1 hunks)apps/assisted-installer-app/src/entry.ts(1 hunks)apps/assisted-installer-app/src/hooks/useInitApp.ts(1 hunks)apps/assisted-installer-app/src/i18n.ts(1 hunks)apps/assisted-installer-app/src/index.html(1 hunks)apps/assisted-installer-app/tsconfig.json(1 hunks)docs/DEVELOPMENT.md(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (24)
- .eslintrc.json
- apps/assisted-installer-app/src/components/Services.tsx
- apps/assisted-installer-app/src/components/ClusterStatus.tsx
- apps/assisted-installer-app/src/bootstrap.ts
- package.json
- apps/assisted-installer-app/src/components/FeatureSupportsLevel.tsx
- apps/assisted-installer-app/src/components/AssistedInstallerExtraDetailCard.tsx
- apps/assisted-installer-app/src/components/RootApp.tsx
- apps/assisted-installer-app/src/components/HostsClusterDetailTab.tsx
- apps/assisted-installer-app/tsconfig.json
- apps/assisted-installer-app/src/components/TechnologyPreview.tsx
- apps/assisted-installer-app/src/entry.ts
- apps/assisted-installer-app/src/components/ExternalLink.tsx
- apps/assisted-installer-app/src/index.html
- apps/assisted-installer-app/src/components/AssistedInstallerDetailCard.tsx
- apps/assisted-installer-app/fec.config.js
- apps/assisted-installer-app/deploy/frontend.yaml
- apps/assisted-installer-app/src/hooks/useInitApp.ts
- apps/assisted-installer-app/package.json
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-push.yaml
- .gitignore
- apps/assisted-installer-app/src/components/NoPermissionsError.tsx
- apps/assisted-installer-app/.tekton/assisted-installer-app-saas-main-pull-request.yaml
- docs/DEVELOPMENT.md
🧰 Additional context used
🪛 Shellcheck (0.10.0)
apps/assisted-installer-app/build_deploy.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 12-12: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
apps/assisted-installer-app/pr_check.sh
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 17-17: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
🪛 Biome (1.9.4)
apps/assisted-installer-app/src/i18n.ts
[error] 53-53: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
|
@rawagner: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
After merging this, we wont need to make a new releases of libraries.
We still need to figure out how to report the version that is currently running & updates to konflux pipelines
Summary by CodeRabbit
New Features
Documentation
Chores