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

Bump required VSCode version to 1.82.3 #503

Merged
merged 10 commits into from
Feb 13, 2024
Merged

Conversation

ybiquitous
Copy link
Member

@ybiquitous ybiquitous commented Feb 13, 2024

Which issue, if any, is this issue related to?

Closes #435

Is there anything in the PR that needs further explanation?

VSCode 1.82.3 included the Electron 25 update. This comes with Node.js 18.15.0.
Ref https://code.visualstudio.com/updates/v1_82

Also, this change removes the Node.js version from the CI testing matrix, following the engines.node field in package.json.


Just to be sure, is Volta still used? If unused, I'd like to remove the settings to achieve DRY.

"volta": {
"node": "14.17.6",
"npm": "8.1.3"
}

VSCode 1.86.1 included the Electron 27 update. This comes with Node.js 18.17.1.
Ref https://code.visualstudio.com/updates/v1_86

Also, this change removes the Node.js version from the CI testing matrix,
following the `engines.node` field in `package.json`.
@ybiquitous ybiquitous added the pr: dependencies relates to dependencies label Feb 13, 2024
const pkg = require('../../package.json');

const vscodeVersion = '1.86.1';
const requiredVscodeVersion = pkg.engines.vscode.match(/\d+\.\d+\.\d+/)?.[0];
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] This file is used as a cache key on CI. So, we must update this file when bumping the VSCode version.

key: ${{ runner.os }}-vscode-${{ hashFiles('test/e2e/jest-runner-vscode.config.js') }}

@ybiquitous ybiquitous changed the title Bump required VSCode version to 1.86.1 Bump required VSCode version to 1.82.3 Feb 13, 2024
@ybiquitous ybiquitous marked this pull request as ready for review February 13, 2024 16:50
@@ -1,6 +1,6 @@
{
"stylelint.customSyntax": "${workspaceFolder}/custom-syntax.js",
"editor.codeActionsOnSave": {
"source.fixAll.stylelint": true
"source.fixAll.stylelint": "explicit"
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] See also #492

Copy link
Member

@adalinesimonian adalinesimonian left a comment

Choose a reason for hiding this comment

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

LGTM

@ybiquitous
Copy link
Member Author

Just to be sure, is Volta still used? If unused, I'd like to remove the settings to achieve DRY.

@adalinesimonian Any thoughts on the comment above?

@adalinesimonian
Copy link
Member

Just to be sure, is Volta still used? If unused, I'd like to remove the settings to achieve DRY.

@adalinesimonian Any thoughts on the comment above?

Yes, it's an nvm alternative that I and other folks use.

@ybiquitous
Copy link
Member Author

Okay. We have to update the Volta settings, right? E.g.

 "volta": { 
-  "node": "14.17.6", 
+  "node": "18.15.0",
   "npm": "8.1.3" 
 }

Also, do we need npm? I don't know which npm version should be used for Volta. npm 9 latest?

@adalinesimonian
Copy link
Member

Okay. We have to update the Volta settings, right?

Yes, correct.

Also, do we need npm? I don't know which npm version should be used for Volta. npm 9 latest?

It's a good practice to pin the npm version as well so that no one runs into weird package manager quirks, same with how we pin our dependencies. 9 is fine.

@ybiquitous
Copy link
Member Author

Thanks. I just updated the Volta settings via 1c1c86f.

@ybiquitous ybiquitous merged commit 0852407 into main Feb 13, 2024
9 checks passed
@ybiquitous ybiquitous deleted the bump-required-vscode-version branch February 13, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: dependencies relates to dependencies
Development

Successfully merging this pull request may close these issues.

2 participants