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

Upgrade node to 18 #6439

Merged
merged 19 commits into from
Mar 4, 2024
Merged

Upgrade node to 18 #6439

merged 19 commits into from
Mar 4, 2024

Conversation

tahmidrahman-dsi
Copy link
Collaborator

@tahmidrahman-dsi tahmidrahman-dsi commented Jan 16, 2024

@tahmidrahman-dsi tahmidrahman-dsi self-assigned this Jan 16, 2024
@tahmidrahman-dsi tahmidrahman-dsi linked an issue Jan 16, 2024 that may be closed by this pull request
2 tasks
Copy link

codecov bot commented Jan 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.33%. Comparing base (58630fd) to head (a1be273).
Report is 405 commits behind head on develop.

❗ Current head a1be273 differs from pull request most recent head 030d38e. Consider uploading reports for the commit 030d38e to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6439      +/-   ##
===========================================
- Coverage    86.66%   86.33%   -0.34%     
===========================================
  Files          731      728       -3     
  Lines       126469   126587     +118     
  Branches     11911    11979      +68     
===========================================
- Hits        109602   109284     -318     
- Misses       16861    17298     +437     
+ Partials         6        5       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@naftis naftis left a comment

Choose a reason for hiding this comment

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

It otherwise works, but I'm getting request to http://localhost:3040/application-config failed, reason: connect ECONNREFUSED ::1:3040 only on this branch for some reason on my Mac. Wonder if it's due to my setup or not. I can access the URL with browser directly

@tahmidrahman-dsi tahmidrahman-dsi changed the base branch from release-v1.3.3 to develop January 22, 2024 06:43
@Zangetsu101
Copy link
Collaborator

I tested on Ubuntu 20.04.6, data seeding worked without any hiccups. I then ran the cypress tests and they also seem to be working fine (albeit a couple of tests are still broken but they are unrelated).

setup.sh Outdated Show resolved Hide resolved
Co-authored-by: Pyry Rouvila <pyry.rouvila@gmail.com>
@rikukissa
Copy link
Collaborator

@tahmidrahman-dsi remember to update CHANGELOG.md

@@ -4,7 +4,7 @@
"description": "OpenCRVS public configuration microservice",
"license": "MPL-2.0",
"scripts": {
"start": "cross-env NODE_ENV=development nodemon --exec ts-node -r tsconfig-paths/register src/index.ts",
"start": "cross-env NODE_ENV=development NODE_OPTIONS=--dns-result-order=ipv4first nodemon --exec ts-node -r tsconfig-paths/register src/index.ts",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"start": "cross-env NODE_ENV=development NODE_OPTIONS=--dns-result-order=ipv4first nodemon --exec ts-node -r tsconfig-paths/register src/index.ts",
"start": "cross-env NODE_ENV=development NODE_OPTIONS=--dns-result-order=ipv4first nodemon --exec ts-node -r tsconfig-paths/register src/index.ts",

Very minor thing but some of these have double spaces in the command

@Zangetsu101
Copy link
Collaborator

@tahmidrahman-dsi I think we can merge this after doing a test by deploying to one of our environments or have we already done that?

@naftis naftis self-requested a review February 6, 2024 13:56
Copy link
Collaborator

@naftis naftis left a comment

Choose a reason for hiding this comment

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

Tested on WSL, works fine.

@euanmillar
Copy link
Collaborator

@tahmidrahman-dsi if you search for "16.20.0" in the code, you need to update .nvmrc, setup.sh, .github/DISCUSSION_TEMPLATE cd-manual-e2e.yml and lint-and-test.yml too please.

CHANGELOG.md Outdated Show resolved Hide resolved
@Zangetsu101 Zangetsu101 enabled auto-merge (squash) March 4, 2024 07:01
@Zangetsu101 Zangetsu101 merged commit d552031 into develop Mar 4, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node upgrade: Core to Node 18 & run dependabot on Hearth
5 participants