chore: upgrade node version #14968
chore: upgrade node version #14968
Conversation
Co-authored-by: Jón Levy <[email protected]>
Co-authored-by: Jón Levy <[email protected]>
Co-authored-by: Jón Levy <[email protected]>
Co-authored-by: Jón Levy <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Jón Levy <[email protected]>
Co-authored-by: Jón Levy <[email protected]>
Co-authored-by: Jón Levy <[email protected]>
|
Affected services are: air-discount-scheme-api,air-discount-scheme-backend,api,application-system-api,download-service,financial-aid-api,financial-aid-backend,financial-aid-open-api,github-actions-cache,icelandic-names-registry-backend,judicial-system-api,judicial-system-digital-mailbox-api,judicial-system-backend,judicial-system-message-handler,judicial-system-robot-api,judicial-system-scheduler,judicial-system-xrd-api,license-api,reference-backend,regulations-admin-backend,services-auth-admin-api,services-auth-delegation-api,services-auth-ids-api,services-auth-personal-representative,services-auth-personal-representative-public,services-auth-public-api,services-contentful-entry-tagger,services-documents,services-endorsements-api,services-search-indexer,services-sessions,services-university-gateway,services-user-notification,services-user-profile,services-xroad-collector,skilavottord-ws,air-discount-scheme-web,auth-admin-web,consultation-portal,contentful-apps,financial-aid-web-osk,financial-aid-web-veita,judicial-system-web,skilavottord-web,web,application-system-form,island-ui-storybook,portals-admin,service-portal,system-e2e,external-contracts-tests, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (3)
infra/scripts/Dockerfile (2)
Line range hint
11-18: ReplaceADDwithCOPYfor better readability and adherence to Docker best practices.- ADD infra/package.json infra/yarn.lock /app/infra/ + COPY infra/package.json infra/yarn.lock /app/infra/ - ADD infra/ /app/infra/ + COPY infra/ /app/infra/ - ADD apps /app/apps/ + COPY apps /app/apps/ - ADD libs /app/libs/ + COPY libs /app/libs/
Line range hint
21-21: Consider consolidating multipleRUNcommands to reduce the number of layers in the Docker image.- RUN ./node_modules/.bin/ncc build src/feature-env.ts -o /app/dist/feature-env - RUN ./node_modules/.bin/ncc build src/secrets.ts -o /app/dist/secrets + RUN ./node_modules/.bin/ncc build src/feature-env.ts -o /app/dist/feature-env && \ + ./node_modules/.bin/ncc build src/secrets.ts -o /app/dist/secretsscripts/ci/Dockerfile (1)
Line range hint
12-13: ReplaceADDwithCOPYfor better clarity and adherence to Docker best practices.- ADD package.json yarn.lock .yarnrc.yml ./ + COPY package.json yarn.lock .yarnrc.yml ./ - ADD apps/native/app/package.json ./apps/native/app/ + COPY apps/native/app/package.json ./apps/native/app/ - ADD . . + COPY . .Also applies to: 25-25
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (14)
- .github/actions/main.ts (1 hunks)
- .github/actions/package.json (1 hunks)
- .github/actions/tsconfig.json (1 hunks)
- .github/actions/unit-test/action.yml (1 hunks)
- .github/actions/yarn.lock (2 hunks)
- .github/workflows/config-values.yaml (2 hunks)
- .github/workflows/pullrequest.yml (8 hunks)
- .github/workflows/push.yml (14 hunks)
- infra/scripts/Dockerfile (1 hunks)
- scripts/ci/10_prepare-docker-deps.sh (2 hunks)
- scripts/ci/Dockerfile (2 hunks)
- scripts/ci/_common.mjs (1 hunks)
- scripts/ci/get-node-modules-hash.mjs (1 hunks)
- scripts/ci/get-node-version.mjs (1 hunks)
Files skipped from review due to trivial changes (3)
- .github/actions/package.json
- .github/actions/tsconfig.json
- .github/actions/yarn.lock
Additional Context Used
Biome (12)
.github/actions/main.ts (12)
8-8: Forbidden non-null assertion.
8-8: Forbidden non-null assertion.
18-18: Forbidden non-null assertion.
19-19: Forbidden non-null assertion.
20-20: Forbidden non-null assertion.
21-21: Forbidden non-null assertion.
27-27: Forbidden non-null assertion.
28-28: Forbidden non-null assertion.
29-29: Forbidden non-null assertion.
33-33: Do not use template literals if interpolation and special-character handling are not needed.
4-6: All these imports are only used as types.
8-8: This let declares a variable that is only assigned once.
Hadolint (30)
infra/scripts/Dockerfile (10)
5: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>
5: Use the
--no-cacheswitch to avoid the need to use--updateand remove/var/cache/apk/*when done installing packages
11: Use COPY instead of ADD for files and folders
12:
COPYto a relative destination withoutWORKDIRset.
16: Use COPY instead of ADD for files and folders
17: Use COPY instead of ADD for files and folders
18: Use COPY instead of ADD for files and folders
21: Multiple consecutive
RUNinstructions. Consider consolidation.
26:
COPYto a relative destination withoutWORKDIRset.
27:
COPYto a relative destination withoutWORKDIRset.scripts/ci/Dockerfile (20)
7: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>
7: Use the
--no-cacheswitch to avoid the need to use--updateand remove/var/cache/apk/*when done installing packages
12: Use COPY instead of ADD for files and folders
13: Use COPY instead of ADD for files and folders
15: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>
16:
yarn cache cleanmissing afteryarn installwas run.
20: Avoid use of wget without progress bar. Use
wget --progress=dot:giga <url>. Or consider using-qor-nv(shorthands for--quietor--no-verbose).
25: Use COPY instead of ADD for files and folders
57: Pin versions in npm. Instead of
npm install <package>usenpm install <package>@<version>
101: Pin versions in apk add. Instead of
apk add <package>useapk add <package>=<version>
101: Use the
--no-cacheswitch to avoid the need to use--updateand remove/var/cache/apk/*when done installing packages
105: Use COPY instead of ADD for files and folders
106: Use COPY instead of ADD for files and folders
107: Use COPY instead of ADD for files and folders
114: Multiple consecutive
RUNinstructions. Consider consolidation.
114: Pin versions in npm. Instead of
npm install <package>usenpm install <package>@<version>
133: Avoid additional packages by specifying
--no-install-recommends
133: Pin versions in apt get install. Instead of
apt-get install <package>useapt-get install <package>=<version>
145: Use COPY instead of ADD for files and folders
166:
COPYto a relative destination withoutWORKDIRset.
Path-based Instructions (5)
scripts/ci/_common.mjs (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
scripts/ci/10_prepare-docker-deps.sh (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
scripts/ci/get-node-modules-hash.mjs (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
scripts/ci/get-node-version.mjs (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
scripts/ci/Dockerfile (1)
Pattern
scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
Additional comments not posted (13)
scripts/ci/_common.mjs (1)
8-11: The implementation ofgetPackageJSONis clean and follows modern JavaScript practices.scripts/ci/10_prepare-docker-deps.sh (1)
Line range hint
11-27: The script effectively uses dynamic Node.js image tags to enhance flexibility in Docker builds..github/actions/unit-test/action.yml (1)
39-41: Updated Node.js setup action to v4 and dynamically set the node version frompackage.json. Ensure that this change integrates well with other workflows and that the specified Node.js version is compatible with all dependencies.scripts/ci/Dockerfile (1)
4-5: IntroducedNODE_IMAGE_TAGto dynamically specify the Node.js version. This aligns with the PR's goal to upgrade Node.js versions..github/workflows/config-values.yaml (1)
64-66: Updated Node.js setup action to v4 and dynamically set the node version frompackage.json. Ensure that this change integrates well with other workflows and that the specified Node.js version is compatible with all dependencies.Also applies to: 112-114
.github/workflows/pullrequest.yml (1)
48-50: Updated Node.js setup action to v4 and dynamically set the node version frompackage.jsonacross multiple jobs. Ensure that this change integrates well with other workflows and that the specified Node.js version is compatible with all dependencies.Also applies to: 83-85, 247-249, 308-310, 371-373, 414-416, 458-460, 507-509
.github/workflows/push.yml (7)
172-174: The update toactions/setup-node@v4and the use ofnode-version-file: 'package.json'aligns with best practices for dynamic Node.js version management.
290-292: Usingget-node-modules-hash.mjsfor calculating the cache key is a good practice as it likely considers multiple factors to ensure a unique and relevant key.
650-650: The use ofNODE_IMAGE_TAGin the Docker build process ensures consistency with the Node.js version used. This is a good practice for maintaining version alignment.
318-320: Setting theTEST_EVERYTHINGenvironment variable based on PR labels is a flexible way to control test execution. However, ensure that the label is being applied as expected.Verification successful
The 'test everything' label is applied correctly in PRs.
- PR with ID 14968 has the 'test everything' label applied.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the 'test everything' label is applied correctly in PRs. # Test: Check for PRs with the label applied. Expect: Correct application of the label. gh pr list --label "test everything"Length of output: 121
239-247: The script to generate theNODE_IMAGE_TAGusingget-node-version.mjsis a robust approach to handle Node.js versioning dynamically. Ensure that the script handles errors gracefully.Verification successful
The
get-node-version.mjsscript includes error handling mechanisms, such as logging errors withconsole.errorand throwing errors withthrow new Error. These mechanisms ensure that errors are reported and handled appropriately.
- The script logs an error message when it fails to get the Docker image for a specific Node.js version.
- It catches errors during the fetch operation and logs a detailed error message.
- It throws an error if it cannot find the Node.js version, ensuring that the failure is propagated and can be handled by the calling process.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in the get-node-version.mjs script. # Test: Check for error handling in the script. Expect: Proper error handling mechanisms. cat scripts/ci/get-node-version.mjs | grep -i 'error'Length of output: 247
51-51: Ensure theNODE_IMAGE_TAGis consistently used across all jobs and steps.Verification successful
The
NODE_IMAGE_TAGis used consistently across various steps and contexts within the.github/workflows/push.ymlfile. It is set and referenced in multiple places, ensuring that the same value is used throughout the workflow.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that NODE_IMAGE_TAG is used consistently across all jobs and steps. # Test: Search for the usage of NODE_IMAGE_TAG. Expect: Consistent usage across the file. rg --type yaml 'NODE_IMAGE_TAG' .github/workflows/push.ymlLength of output: 1465
386-386: The script10_prepare-docker-deps.shis crucial for preparing Docker dependencies. Ensure it is optimized for performance and correctly handles all necessary dependencies.Verification successful
The
10_prepare-docker-deps.shscript, along with its dependencies (_common.shandDockerfile), appears to be well-structured and optimized for performance. The script sets strict error handling, ensures necessary directories are created, dynamically determines the Node.js image tag, and uses Docker Buildx with caching to optimize builds. The_common.shscript sets up environment variables and project root, while theDockerfilehandles various stages of the build process, including dependency installation and caching.
- The
10_prepare-docker-deps.shscript is optimized for performance with caching mechanisms.- The
_common.shscript correctly handles necessary environment variables and project setup.- The
Dockerfileincludes all necessary dependencies and stages for building the Docker images.No further issues were found regarding performance optimization and dependency handling.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the contents and performance of the 10_prepare-docker-deps.sh script. # Test: Check the script for performance optimization and dependency handling. Expect: Optimized performance and complete dependency handling. cat scripts/ci/10_prepare-docker-deps.shLength of output: 864
Script:
#!/bin/bash # Description: Verify the contents of the _common.sh script and the Dockerfile used in 10_prepare-docker-deps.sh. # Test: Check the _common.sh script for dependency handling. echo "Contents of _common.sh:" cat scripts/ci/_common.sh # Test: Check the Dockerfile for dependency handling. echo "Contents of Dockerfile:" cat scripts/ci/DockerfileLength of output: 4682
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14968 +/- ##
=======================================
Coverage 37.13% 37.13%
=======================================
Files 6373 6373
Lines 129702 129701 -1
Branches 36999 37000 +1
=======================================
+ Hits 48160 48161 +1
+ Misses 81542 81540 -2 Flags with carried forward coverage won't be shown. Click here to find out more. see 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 99 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (9)
|
lodmfjord
added
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/push.yml (15 hunks)
- scripts/ci/_nx-affected-targets.sh (1 hunks)
Files skipped from review due to trivial changes (1)
- scripts/ci/_nx-affected-targets.sh
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/push.yml
This reverts commit a40e348.
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
NODE_IMAGE_TAGfor Docker builds.Updates
actions/setup-nodefromv2tov4in various workflows.esnextfor better compatibility.Bug Fixes
Chores
@types/nodepackage to version20.12.12.