-
Notifications
You must be signed in to change notification settings - Fork 111
Consume VS Code built-in extensions in Che-Theia #618
Conversation
crw-ci-test |
15f1672
to
3224b55
Compare
generator/src/foreach_yarn
Outdated
@@ -45,3 +49,14 @@ function mkdirSyncRecursive(dir) { | |||
curPath.length > 0 && !fs.existsSync(curPath) ? fs.mkdirSync(curPath) : null; | |||
} | |||
}; | |||
|
|||
function copyFolderSync(from, to) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hello, is it fast to copy all the plugins ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean, in comparison with calling system cp -r
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes sorry I didn't write much comment
yes comparing this nodejs copy vs system cp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't checked yet. I can try to compare these two approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comparison provided in: #618 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would wait until monday to merge if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're copying too many extensions
for example vscode-builtin-typescript-language-features (typescript language server is only added if a sidecar/different plug-in is added no ) ?
Also what about git ? shouldn't it be part of the built-in extensions instead of being copied from Dockerfile ?
https://github.com/theia-ide/vscode-builtin-extensions/releases/tag/v1.39.1-prel
If I understand everything right, we should not just copy the set of all the VS Code extensions that Theia includes. If that's correct, we can just reuse the mechanism provided by Theia:
|
@azatsarynnyy maybe that yes if we're depending on a custom set of extensions it'll be easier It's mainly because we decided to move some extensions to other containers Also probably we need to remove https://github.com/eclipse/che-theia/blob/master/dockerfiles/theia/Dockerfile#L99 and use it from https://github.com/theia-ide/vscode-builtin-extensions/releases as well (through package.json |
@benoitf we need to take into account, that if we use |
crw-ci-test |
@benoitf I compared two ways of copying the directories: called command to measure the execution time and copied 46 extensions: nodejs copy with function:
./foreach_yarn 0.12s user 0.17s system 55% cpu 0.524 total system copy with the function:
./foreach_yarn 0.14s user 0.32s system 81% cpu 0.555 total So, copying using nodejs a bit faster, as it doesn't creates additional process. |
crw-ci-test |
✅ E2E Happy path tests succeed 🎉 See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) |
✅ E2E Happy path tests succeed 🎉 See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) |
✅ E2E Happy path tests succeed 🎉 See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) |
✅ E2E Happy path tests succeed 🎉 See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) |
"vscode-builtin-shellscript": "https://github.com/theia-ide/vscode-builtin-extensions/releases/download/v1.39.1-prel/shellscript-1.39.1-prel.vsix", | ||
"vscode-builtin-sql": "https://github.com/theia-ide/vscode-builtin-extensions/releases/download/v1.39.1-prel/sql-1.39.1-prel.vsix", | ||
"vscode-builtin-swift": "https://github.com/theia-ide/vscode-builtin-extensions/releases/download/v1.39.1-prel/swift-1.39.1-prel.vsix", | ||
"vscode-builtin-typescript": "https://github.com/theia-ide/vscode-builtin-extensions/releases/download/v1.39.1-prel/typescript-1.39.1-prel.vsix", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess typescript extension can be removed as we're providing Che Plugin instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the updated image and there's no syntax highlighting for TS.
As I understand, there're two builtin extensions in VS Code: typescript-basics and typescript-language-features.
What we provide with TS Che Plugin it's typescript-language-features
. And typescript-basics
must be bundled with Che Theia.
So, let's revert the removed one - vscode-builtin-typescript
. Sorry for the confusion.
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested on che.openshift.io
crw-ci-test |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
crw-ci-test |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
crw-ci-test |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
Signed-off-by: Vladyslav Zhukovskyi <vzhukovs@redhat.com>
d4ba87d
to
c75d401
Compare
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
crw-ci-test |
@vzhukovskii |
@RomanNikitenko the latest run was unstable, error message found in logs:
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
crw-ci-test |
✅ E2E Happy path tests succeed 🎉 See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) |
What does this PR do?
This changes proposal include in
che-theia
image default VS Code built-in extensions that provide by theia build.To check this PR, you can use following devfile:
When workspace will start, you'll be able to open
View
-Open View...
-Plugins
to ensure that all VS Code built-in extensions are included into assembly.Signed-off-by: Vladyslav Zhukovskyi vzhukovs@redhat.com
What issues does this PR fix or reference?
eclipse-che/che#14759
Release Notes
Consume VS Code built-in extensions in Che-Theia
Docs PR
N/A - internal enhancement