Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: upgrade node version #14985

Merged
merged 80 commits into from
May 30, 2024
Merged

chore: upgrade node version #14985

merged 80 commits into from
May 30, 2024

Conversation

lodmfjord
Copy link
Member

@lodmfjord lodmfjord commented May 29, 2024

...

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:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

  • New Features

    • Introduced NODE_IMAGE_TAG for dynamic Node.js versioning in Docker builds.
    • Added new script to calculate Node module hashes for improved caching.
  • Updates

    • Node.js setup in GitHub Actions updated to v4 for better compatibility.
    • Type definitions for Node.js updated from 20.11.4 to 20.12.12.
  • Improvements

    • Enhanced Docker build process with dynamic Node.js versioning.
    • Refactored scripts for better workflow control and efficiency.
  • Bug Fixes

    • Improved logic for determining revisions based on environment variables and event names.

lodmfjord and others added 30 commits April 12, 2024 11:17
Co-authored-by: Jón Levy <levy@andes.is>
Co-authored-by: Jón Levy <levy@andes.is>
Co-authored-by: Jón Levy <levy@andes.is>
Co-authored-by: Jón Levy <levy@andes.is>
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 <levy@andes.is>
Co-authored-by: Jón Levy <levy@andes.is>
Co-authored-by: Jón Levy <levy@andes.is>
@datadog-island-is
Copy link

datadog-island-is bot commented May 29, 2024

Datadog Report

All test runs ba3fd41 🔗

101 Total Test Services: 0 Failed, 99 Passed
🔻 Test Sessions change in coverage: 10 decreased, 8 increased, 93 no change

Test Services
This report shows up to 10 services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
air-discount-scheme-backend 0 0 0 81 0 42.96s N/A Link
air-discount-scheme-web 0 0 0 2 0 10.68s N/A Link
api 0 0 0 4 0 4.99s N/A Link
api-catalogue-services 0 0 0 23 0 16.93s N/A Link
api-domains-air-discount-scheme 0 0 0 6 0 38.98s N/A Link
api-domains-assets 0 0 0 3 0 25.19s N/A Link
api-domains-auth-admin 0 0 0 18 0 13.23s N/A Link
api-domains-communications 0 0 0 5 0 44.78s N/A Link
api-domains-criminal-record 0 0 0 5 0 15.67s N/A Link
api-domains-driving-license 0 0 0 23 0 57.88s N/A Link

🔻 Code Coverage Decreases vs Default Branch (10)

This report shows up to 5 code coverage decreases.

  • services-auth-ids-api - jest 49.78% (-1.41%) - Details
  • clients-syslumenn - jest 42.21% (-0.62%) - Details
  • file-storage - jest 55.07% (-0.38%) - Details
  • feature-flags - jest 91.66% (-0.23%) - Details
  • judicial-system-xrd-api - jest 74.32% (-0.21%) - Details

Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.13%. Comparing base (5452a2c) to head (612e927).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #14985      +/-   ##
==========================================
+ Coverage   37.12%   37.13%   +0.01%     
==========================================
  Files        6390     6390              
  Lines      130064   130002      -62     
  Branches    37107    37074      -33     
==========================================
  Hits        48282    48282              
+ Misses      81782    81720      -62     
Flag Coverage Δ
api 3.46% <ø> (ø)
api-catalogue-services 77.85% <ø> (ø)
api-domains-health-insurance 27.15% <ø> (ø)
application-system-api 41.97% <ø> (ø)
application-template-api-modules 24.28% <ø> (+0.03%) ⬆️
application-templates-accident-notification 19.67% <ø> (ø)
application-templates-driving-license 16.47% <ø> (ø)
application-templates-estate 11.79% <ø> (ø)
auth-nest-tools 30.44% <ø> (ø)
auth-react 23.00% <ø> (ø)
clients-financial-statements-inao 48.40% <ø> (ø)
clients-license-client 1.84% <ø> (ø)
clients-middlewares 73.36% <ø> (-0.04%) ⬇️
content-search-index-manager 95.65% <ø> (ø)
nest-audit 68.20% <ø> (ø)
nest-config 77.27% <ø> (ø)
nest-problem 46.08% <ø> (ø)
nest-sequelize 94.44% <ø> (ø)
portals-admin-regulations-admin 2.06% <ø> (ø)
residence-history 85.00% <ø> (ø)
services-user-profile 62.72% <ø> (ø)
shared-utils 26.14% <ø> (ø)
skilavottord-ws 23.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 9 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b65f614...612e927. Read the comment docs.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
.github/actions/main.ts (2)

Line range hint 15-15: Refactor to avoid non-null assertions to enhance code safety.

- let git = new SimpleGit(process.env.REPO_ROOT!, process.env.SHELL!)
+ let git = new SimpleGit(process.env.REPO_ROOT, process.env.SHELL)
+ if (!process.env.REPO_ROOT || !process.env.SHELL) {
+   throw new Error('Required environment variables REPO_ROOT or SHELL are not set.');
+ }

Also applies to: 24-24, 25-25, 26-26, 27-27, 33-33, 34-34, 35-35


Line range hint 4-5: Consider importing only the necessary parts of modules to optimize bundle size.

- import { LocalRunner } from './ci-io'
- import { findBestGoodRefBranch, findBestGoodRefPR } from './change-detection'
- import { Octokit } from '@octokit/action'
- import { SimpleGit } from './simple-git'
- import { WorkflowID } from './git-action-status'
+ import type { LocalRunner } from './ci-io'
+ import type { findBestGoodRefBranch, findBestGoodRefPR } from './change-detection'
+ import type { Octokit } from '@octokit/action'
+ import type { SimpleGit } from './simple-git'
+ import type { WorkflowID } from './git-action-status'
Review Details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 84266f6 and 4eb8ba3.
Files selected for processing (6)
  • .github/actions/change-detection.ts (1 hunks)
  • .github/actions/dist/main.js (2 hunks)
  • .github/actions/main.ts (1 hunks)
  • .github/workflows/pullrequest.yml (8 hunks)
  • .github/workflows/push.yml (14 hunks)
  • scripts/ci/_nx-affected-targets.sh (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/pullrequest.yml
  • .github/workflows/push.yml
  • scripts/ci/_nx-affected-targets.sh
Additional Context Used
Biome (21)
.github/actions/change-detection.ts (10)

77-77: This type annotation is trivially inferred from its initialization.


116-204: Promise executor functions should not be async.


139-139: Do not use template literals if interpolation and special-character handling are not needed.


141-141: Do not use template literals if interpolation and special-character handling are not needed.


191-191: Do not use template literals if interpolation and special-character handling are not needed.


193-193: Do not use template literals if interpolation and special-character handling are not needed.


202-202: Do not use template literals if interpolation and special-character handling are not needed.


1-1: All these imports are only used as types.


1-2: All these imports are only used as types.


174-174: This let declares a variable that is only assigned once.

.github/actions/main.ts (11)

15-15: Forbidden non-null assertion.


15-15: Forbidden non-null assertion.


24-24: Forbidden non-null assertion.


25-25: Forbidden non-null assertion.


26-26: Forbidden non-null assertion.


27-27: Forbidden non-null assertion.


33-33: Forbidden non-null assertion.


34-34: Forbidden non-null assertion.


35-35: Forbidden non-null assertion.


4-5: All these imports are only used as types.


15-15: This let declares a variable that is only assigned once.

.github/actions/change-detection.ts Outdated Show resolved Hide resolved
.github/actions/change-detection.ts Outdated Show resolved Hide resolved
@lodmfjord lodmfjord added deploy-feature Deploys features to dev test everything 💀 test & build everything 💀 labels May 30, 2024
Copy link
Contributor

github-actions bot commented May 30, 2024

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,

Copy link
Member

@AndesKrrrrrrrrrrr AndesKrrrrrrrrrrr left a comment

Choose a reason for hiding this comment

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

LGTM

scripts/ci/_nx-affected-targets.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range and nitpick comments (3)
.github/actions/main.ts (3)

Line range hint 15-15: Refactor to avoid non-null assertions and improve readability.

- let git = new SimpleGit(process.env.REPO_ROOT!, process.env.SHELL!)
+ let git = new SimpleGit(process.env.REPO_ROOT, process.env.SHELL)
+ if (!process.env.REPO_ROOT || !process.env.SHELL) {
+   throw new Error('Required environment variables REPO_ROOT or SHELL are not set.');
+ }

Line range hint 24-27: Refactor to avoid non-null assertions in findBestGoodRefPR and findBestGoodRefBranch calls.

-          `'${process.env.HEAD_REF!}'`,
-          `'${process.env.BASE_REF!}'`,
-          `'${process.env.PR_REF!}'`,
-          process.env.WORKFLOW_ID! as WorkflowID,
+          process.env.HEAD_REF ? `'${process.env.HEAD_REF}'` : throw new Error('HEAD_REF is not set'),
+          process.env.BASE_REF ? `'${process.ENV.BASE_REF}'` : throw new Error('BASE_REF is not set'),
+          process.env.PR_REF ? `'${process.env.PR_REF}'` : throw new Error('PR_REF is not set'),
+          process.env.WORKFLOW_ID ? process.env.WORKFLOW_ID as WorkflowID : throw new Error('WORKFLOW_ID is not set'),

Line range hint 33-35: Refactor to avoid non-null assertions in rev.branch and rev.ref assignments.

-  rev.branch = rev.branch.replace(/'/g, '')
-  rev.ref = rev.ref.replace(/'/g, '')
+  if (rev && rev.branch && rev.ref) {
+    rev.branch = rev.branch.replace(/'/g, '')
+    rev.ref = rev.ref.replace(/'/g, '')
+  } else {
+    throw new Error('Revision data is incomplete or not set.');
+  }
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4eb8ba3 and c95bb28.

Files selected for processing (4)
  • .github/actions/dist/main.js (2 hunks)
  • .github/actions/main.ts (1 hunks)
  • .github/workflows/push.yml (15 hunks)
  • scripts/ci/_nx-affected-targets.sh (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • .github/workflows/push.yml
  • scripts/ci/_nx-affected-targets.sh
Additional context used
Biome
.github/actions/main.ts

[error] 15-15: Forbidden non-null assertion.


[error] 15-15: Forbidden non-null assertion.


[error] 24-24: Forbidden non-null assertion.


[error] 25-25: Forbidden non-null assertion.


[error] 26-26: Forbidden non-null assertion.


[error] 27-27: Forbidden non-null assertion.


[error] 33-33: Forbidden non-null assertion.


[error] 34-34: Forbidden non-null assertion.


[error] 35-35: Forbidden non-null assertion.


[error] 4-5: All these imports are only used as types.


[error] 15-15: This let declares a variable that is only assigned once.

Additional comments not posted (1)
.github/actions/main.ts (1)

10-10: Ensure that the environment variables are properly set before proceeding.

Verification successful

The environment variables NX_AFFECTED_ALL and TEST_EVERYTHING are properly set in the GitHub Actions workflow files (push.yml and pullrequest.yml). This ensures that the condition in the script at .github/actions/main.ts will function as intended.

  • .github/workflows/push.yml
  • .github/workflows/pullrequest.yml
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the environment variables `NX_AFFECTED_ALL` and `TEST_EVERYTHING` are set in the GitHub Actions workflow files.

# Test: Search for the environment variable setup in workflow files. Expect: Proper setup of these variables.
rg --type yaml 'NX_AFFECTED_ALL|TEST_EVERYTHING' .github/workflows/

Length of output: 942

coderabbitai[bot]
coderabbitai bot previously requested changes May 30, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range and nitpick comments (2)
.github/actions/main.ts (2)

Line range hint 18-18: Refactor to avoid non-null assertions and improve code safety.

- let git = new SimpleGit(process.env.REPO_ROOT!, process.env.SHELL!)
+ const git = new SimpleGit(process.env.REPO_ROOT, process.env.SHELL)
+ if (!process.env.REPO_ROOT || !process.env.SHELL) {
+   throw new Error('Required environment variables REPO_ROOT or SHELL are not set.');
+ }

Also applies to: 27-30, 36-38


Line range hint 36-38: Optimize the replacement of single quotes in rev object properties.

- rev.branch = rev.branch.replace(/'/g, '')
- rev.ref = rev.ref.replace(/'/g, '')
+ rev.branch = rev.branch.replaceAll("'", "")
+ rev.ref = rev.ref.replaceAll("'", "")
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c95bb28 and 1fb7a74.

Files selected for processing (2)
  • .github/actions/main.ts (1 hunks)
  • .github/workflows/push.yml (14 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/push.yml
Additional context used
Biome
.github/actions/main.ts

[error] 18-18: Forbidden non-null assertion.


[error] 18-18: Forbidden non-null assertion.


[error] 27-27: Forbidden non-null assertion.


[error] 28-28: Forbidden non-null assertion.


[error] 29-29: Forbidden non-null assertion.


[error] 30-30: Forbidden non-null assertion.


[error] 36-36: Forbidden non-null assertion.


[error] 37-37: Forbidden non-null assertion.


[error] 38-38: Forbidden non-null assertion.


[error] 4-5: All these imports are only used as types.


[error] 18-18: This let declares a variable that is only assigned once.

Additional comments not posted (1)
.github/actions/main.ts (1)

10-13: LGTM! Verify that the environment variables NX_AFFECTED_ALL and TEST_EVERYTHING are correctly set in all deployment configurations.

Verification successful

The environment variables NX_AFFECTED_ALL and TEST_EVERYTHING are correctly set in the deployment configurations.

  • .github/workflows/push.yml
  • .github/workflows/pullrequest.yml
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the environment variables are set in all deployment configurations.

# Test: Search for environment variable settings in deployment configurations. Expect: Proper settings in all configurations.
rg --type yaml 'NX_AFFECTED_ALL|TEST_EVERYTHING' .github/workflows/

Length of output: 942

.github/actions/main.ts Show resolved Hide resolved
@lodmfjord lodmfjord added the automerge Merge this PR as soon as all checks pass label May 30, 2024
@kodiakhq kodiakhq bot merged commit c4f12fb into main May 30, 2024
494 checks passed
@kodiakhq kodiakhq bot deleted the upgrade-node-version branch May 30, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this PR as soon as all checks pass deploy-feature Deploys features to dev test everything 💀 test & build everything 💀
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants