chore: migrate to pnpm#229
Conversation
|
Thanks Denis. For app-4 in your screenshot, which test case (repo) did you use and how did you create/deploy it? |
I created a simple file 
|
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR migrates the build and dependency management tooling from Yarn to pnpm across Containerfile.plugin, Dockerfile, package.json scripts, and README.md instructions. It also updates package.json dependencies/plugin config, adds npmrc hoist patterns, and changes webpack's CSS exclusion logic. ChangesYarn to pnpm migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Related PRs: None identified. Suggested labels: dependencies, build Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.npmrc (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBroaden-then-narrow:
*types*hoist pattern is too permissive.
*types*matches any package name containing that substring anywhere (e.g.mime-types), not just@types/*scoped packages. This can unintentionally hoist unrelated transitive deps, reintroducing phantom-dependency issues.Proposed tightening
-public-hoist-pattern[]=*types* +public-hoist-pattern[]=`@types/`*🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.npmrc at line 1, The public-hoist-pattern entry is too broad because the *types* glob can match unrelated packages like mime-types, so tighten the hoist rule in .npmrc to target only `@types` scoped packages. Update the existing hoist pattern to a more specific matcher using the same npm config entry so only intended TypeScript type packages are hoisted and phantom dependencies are avoided.package.json (1)
86-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale
jest/@types/jestoverrides after removing Jest scripts.
pnpm.overridesstill pinsjest/@types/jestto the very old21.xline even though the Jest test scripts were removed fromscripts(lines 8-17). If Jest is no longer used, these overrides are dead weight pinning an outdated (and likely vulnerable) transitive version; if it's still needed transitively, the override should track a current major version instead of21.x.#!/bin/bash # Check if jest is referenced anywhere else in the repo (config, CI, other scripts) rg -n "jest" --iglob '!**/node_modules/**'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 86 - 100, The pnpm overrides still pin jest and `@types/jest` to the obsolete 21.x line even though the Jest scripts were removed from the package scripts. Update the package.json overrides by removing these entries if Jest is no longer used anywhere, or bump them to a currently supported major if they are still required transitively; check the package.json scripts and any other Jest references in the repo before deciding. Use the existing pnpm.overrides block and the jest/@types/jest keys to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.konflux/Containerfile.plugin:
- Around line 21-32: The container build in Containerfile.plugin uses an
unpinned pnpm install and disables lockfile enforcement, so update the build to
use a pinned pnpm version via Corepack or an explicit version tied to
packageManager, and switch both pnpm install steps back to frozen-lockfile
behavior. Adjust the install flow around the pnpm setup and the two pnpm install
commands so the image build always follows the committed pnpm-lock.yaml and does
not drift with newer pnpm releases.
In `@Dockerfile`:
- Around line 5-6: The build stage is invoking pnpm without first making pnpm
available, so update the Dockerfile’s setup before the RUN pnpm config set and
RUN pnpm install/build steps by enabling corepack or installing pnpm in this
node:23 stage. Also correct the pnpm timeout configuration in the same
Dockerfile setup from network-timeout to fetchTimeout so the configuration is
recognized.
In `@package.json`:
- Around line 8-17: The package manifest no longer exposes the test-related
scripts expected by the documentation, so the contributor workflow is broken. In
package.json, restore the missing test, test:update, and test:coverage entries
if they are still supported, or update the README Testing section to reference
the current script names instead of yarn test*. Keep the commands aligned with
the existing npm scripts alongside build, start, and lint.
In `@webpack.config.ts`:
- Around line 50-53: The allow-list regex in the webpack config is checking the
wrong package scope, so `@openshift-console/plugin-shared` is still being
excluded from the sass/css loader. Update the `exclude` predicate in
`webpack.config.ts` to match the actual dependency name used by
`@openshift-console/plugin-shared` (alongside the existing `@patternfly`
exception), so the intended package is included correctly.
---
Nitpick comments:
In @.npmrc:
- Line 1: The public-hoist-pattern entry is too broad because the *types* glob
can match unrelated packages like mime-types, so tighten the hoist rule in
.npmrc to target only `@types` scoped packages. Update the existing hoist pattern
to a more specific matcher using the same npm config entry so only intended
TypeScript type packages are hoisted and phantom dependencies are avoided.
In `@package.json`:
- Around line 86-100: The pnpm overrides still pin jest and `@types/jest` to the
obsolete 21.x line even though the Jest scripts were removed from the package
scripts. Update the package.json overrides by removing these entries if Jest is
no longer used anywhere, or bump them to a currently supported major if they are
still required transitively; check the package.json scripts and any other Jest
references in the repo before deciding. Use the existing pnpm.overrides block
and the jest/@types/jest keys to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: c2cdaacf-7d49-4e2b-8034-6ce11f3edfe8
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.konflux/Containerfile.plugin.npmrcDockerfileREADME.mdpackage.jsonwebpack.config.ts
| # Install pnpm | ||
| RUN npm install --global pnpm | ||
|
|
||
| COPY ["package.json", "yarn.lock", "./"] | ||
| COPY ["package.json", "pnpm-lock.yaml", "./"] | ||
|
|
||
| RUN yarn install --no-progress --non-interactive | ||
| RUN pnpm install --no-frozen-lockfile --ignore-scripts | ||
|
|
||
| ADD . /usr/src/app | ||
| WORKDIR /usr/src/app | ||
|
|
||
| RUN yarn install --network-timeout 600000 && \ | ||
| yarn cache clean | ||
|
|
||
| RUN yarn install --no-progress --non-interactive | ||
| RUN pnpm install --no-frozen-lockfile && \ | ||
| pnpm store prune |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "packageManager" package.jsonRepository: redhat-developer/gitops-console-plugin
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package.json ==\n'
sed -n '1,120p' package.json
printf '\n== pnpm-lock.yaml head ==\n'
sed -n '1,40p' pnpm-lock.yaml
printf '\n== .konflux/Containerfile.plugin ==\n'
nl -ba .konflux/Containerfile.plugin | sed -n '1,80p'Repository: redhat-developer/gitops-console-plugin
Length of output: 5151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether corepack or pnpm version pinning is already used elsewhere
rg -n "corepack|pnpm@|packageManager|frozen-lockfile|no-frozen-lockfile" -S .Repository: redhat-developer/gitops-console-plugin
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect lockfile version semantics without executing repo code
python3 - <<'PY'
from pathlib import Path
import re
text = Path('pnpm-lock.yaml').read_text()
m = re.search(r'^lockfileVersion:\s*(.+)$', text, re.M)
print('lockfileVersion:', m.group(1).strip() if m else 'MISSING')
PYRepository: redhat-developer/gitops-console-plugin
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .konflux/Containerfile.plugin ==\n'
sed -n '1,80p' .konflux/Containerfile.plugin
printf '\n== pnpm pinning / corepack references ==\n'
grep -RInE 'corepack|pnpm@|packageManager|frozen-lockfile|no-frozen-lockfile' . --exclude-dir=.git || trueRepository: redhat-developer/gitops-console-plugin
Length of output: 50394
Pin pnpm and restore frozen installs in the container build.
package.jsonhas nopackageManagerpin, sonpm install --global pnpmcan drift to a different pnpm release over time.- Both
pnpm installsteps use--no-frozen-lockfile, which allows dependency resolution to move away from the committedpnpm-lock.yaml.
Use a pinned pnpm version (or Corepack) and switch back to frozen-lockfile behavior for the image build.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.konflux/Containerfile.plugin around lines 21 - 32, The container build in
Containerfile.plugin uses an unpinned pnpm install and disables lockfile
enforcement, so update the build to use a pinned pnpm version via Corepack or an
explicit version tied to packageManager, and switch both pnpm install steps back
to frozen-lockfile behavior. Adjust the install flow around the pnpm setup and
the two pnpm install commands so the image build always follows the committed
pnpm-lock.yaml and does not drift with newer pnpm releases.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #229 +/- ##
==========================================
- Coverage 11.92% 11.84% -0.09%
==========================================
Files 154 154
Lines 6272 6326 +54
Branches 2028 2108 +80
==========================================
+ Hits 748 749 +1
- Misses 5524 5577 +53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: dkarpele <karpelevich@gmail.com>
3 participants
I tested commands from Readme.md (Option 1) and package.json: