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:update node version to v18 #26358

Merged
merged 3 commits into from
Aug 15, 2023
Merged

chore:update node version to v18 #26358

merged 3 commits into from
Aug 15, 2023

Conversation

KelvinOm
Copy link
Collaborator

Description

Update node version

@KelvinOm KelvinOm added skip-changelog Adding this label to a PR prevents it from being listed in the changelog skip-docs skip-testPlan labels Aug 15, 2023
@KelvinOm KelvinOm marked this pull request as draft August 15, 2023 10:09
@@ -5,5 +5,7 @@ plugins:
spec: "https://raw.githubusercontent.com/ambar/yarn-plugin-dedupe-on-install/main/index.js"
- path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs
spec: "@yarnpkg/plugin-workspace-tools"
- path: .yarn/plugins/@yarnpkg/plugin-engines.cjs
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yarn berry needs a separate plugin to check the engines versions.

yarnpkg/berry#1177

@@ -3,8 +3,8 @@
"version": "0.1.0",
"private": true,
"engines": {
"node": "^16.14.0",
"npm": "^8.5.5"
"node": "^18.17.1",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Latest LTS version for this moment.

@ghost
Copy link

ghost commented Aug 15, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@KelvinOm KelvinOm changed the title chore:update node version chore:update node version to v18 Aug 15, 2023
@appsmithorg appsmithorg deleted a comment from github-actions bot Aug 15, 2023
@KelvinOm
Copy link
Collaborator Author

The build with the tests is here.

@KelvinOm KelvinOm marked this pull request as ready for review August 15, 2023 12:03
@sharat87
Copy link
Member

@KelvinOm @riodeuno, in addition to this, should we also consider changing the node-version: a.b.c lines to node-version-file: app/client/package.json instead? This will give us two things:

  1. The version number is not duplicated a billion times all across the source code.
  2. The NodeJS version is taken from the current branch, instead of the release branch for ok-to-test.

This is supported by code here.

I've tested in a separate test repo and it works.

shot-2023-08-15-12-19-13

What do you think?

riodeuno
riodeuno previously approved these changes Aug 15, 2023
@riodeuno
Copy link
Contributor

@sharat87 Yes, I agree. @KelvinOm do you see any challenges in using this?

@KelvinOm
Copy link
Collaborator Author

@sharat87 Yes, I agree. @KelvinOm do you see any challenges in using this?

I think it's great! Checking...

@KelvinOm
Copy link
Collaborator Author

/ok-to-test

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5867116039.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 26358.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=26358&runId=5867116039_1

@sharat87
Copy link
Member

Oh ok-to-test will still use v16, since the workflow changes to use package.json need to be in the release branch. 🙂

@KelvinOm KelvinOm requested a review from riodeuno August 15, 2023 12:42
@KelvinOm
Copy link
Collaborator Author

KelvinOm commented Aug 15, 2023

The build with the tests is here. I think we can not wait for this. Since the last run was green, and here we just fixed the scripts for the node version that works.

Снимок экрана 2023-08-15 в 15 44 11

@sharat87 sharat87 merged commit 826d58f into release Aug 15, 2023
77 of 79 checks passed
@sharat87 sharat87 deleted the chore/update-node-version branch August 15, 2023 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Adding this label to a PR prevents it from being listed in the changelog skip-docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants