chore(infra): Upgrade helm build yarn #16890
Conversation
This reverts commit 33b6545.
WalkthroughThe pull request introduces updates to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
infra/scripts/Dockerfile (1)
The
--openssl-legacy-providerflag can be safely removedThe project is using Node.js 20.15.0 as specified in package.json, which doesn't require the legacy OpenSSL provider. This flag was primarily needed for Node.js 17+ to support older versions of webpack and related tools, but Node.js 20.x has resolved these compatibility issues. The webpack configurations in the project don't show any specific SSL requirements that would necessitate this legacy provider.
- Remove
ENV NODE_OPTIONS=--openssl-legacy-providerfrominfra/scripts/Dockerfile🔗 Analysis chain
Line range hint
6-6: Review the need for legacy OpenSSL provider.The environment variable
NODE_OPTIONS=--openssl-legacy-providermight be unnecessary with newer Node.js versions. Consider reviewing if this flag is still required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for OpenSSL-related configurations and discussions # Expected: Find context about why legacy provider is needed # Search for OpenSSL-related configurations rg -A 5 "openssl-legacy-provider" # Search for related discussions in documentation rg -l "openssl" docs/ README.mdLength of output: 636
Script:
#!/bin/bash # Let's check the Node.js version specified in the Dockerfile and package.json # to determine if legacy OpenSSL provider is needed # Check Dockerfile for Node version rg -A 2 "FROM node:" infra/scripts/Dockerfile # Check package.json for Node version requirements cat infra/package.json | grep -A 5 '"engines"' # Check for any webpack configurations that might require legacy SSL fd -e js -e json webpackLength of output: 915
Script:
#!/bin/bash # Let's check webpack configurations for any specific SSL requirements # that might necessitate the legacy provider # Check webpack configs for SSL-related configurations rg -A 5 "https" apps/*/webpack.config.js tools/*/webpack.config.js # Also check for any SSL/TLS related configurations rg -A 5 "ssl|tls" apps/*/webpack.config.js tools/*/webpack.config.js # Check package-lock.json or yarn.lock for webpack version cat infra/yarn.lock | grep -A 2 "webpack@"Length of output: 1628
infra/package.json (1)
6-6: Consider using a more specific npm restrictionWhile
"please-use-yarn"effectively prevents npm usage, consider using a more standard approach like">=0.0.0"with documentation explaining the Yarn requirement.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
infra/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
infra/package.json(2 hunks)infra/scripts/Dockerfile(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (3)
infra/scripts/Dockerfile (2)
15-15: Good addition of diagnostic commands for build visibility.
The addition of diagnostic commands (ls, id, pwd, cat) will help in debugging build issues by providing clear context about the build environment.
15-15: Verify corepack availability in the base image.
The addition of corepack enable is correct for managing Yarn versions, but we should verify its availability in the base image.
#!/bin/bash
# Description: Check if corepack is available in the specified Node image
# Expected: Find evidence of corepack in the image
# Extract the Node image tag from the Dockerfile
NODE_IMAGE_TAG=$(grep "ARG NODE_IMAGE_TAG" infra/scripts/Dockerfile)
# Search for any documentation about Node versions and corepack
rg -l "corepack" README.md docs/infra/package.json (1)
36-36: LGTM: Type definitions addition
The addition of @types/yargs matches the existing yargs dependency and provides proper TypeScript support.
AndesKrrrrrrrrrrr
added
deploy-feature
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16890 +/- ##
=======================================
Coverage 36.41% 36.41%
=======================================
Files 6898 6898
Lines 144868 144873 +5
Branches 41403 41406 +3
=======================================
+ Hits 52751 52757 +6
+ Misses 92117 92116 -1 Flags with carried forward coverage won't be shown. Click here to find out more. see 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ❌ 99 Total Test Services: 1 Failed, 96 Passed Test ServicesThis report shows up to 10 services
❌ Failed Tests (11)
🔻 Code Coverage Decreases vs Default Branch (16)
|
This reverts commit d81308c.
* Add engines to infra/package.json * Add packageManager to pakage.json * Move packageManager next to engines in root package.json * Remove hideBin import * Revert "Remove hideBin import" This reverts commit 33b6545. * Add typing package for yargs * Enable corepack before yarn install * Set volta for infra/package.json --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
The yarn version used in the
infra/folder was not sepecified, so thedocker-build-helmjob defaulted to version1.22.Upgrading for modernity, and determinism.
Summary by CodeRabbit
New Features
enginesandpackageManager.Chores