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

Package vsix #39

Merged
merged 14 commits into from
Oct 30, 2023
Merged

Package vsix #39

merged 14 commits into from
Oct 30, 2023

Conversation

deribaucourt
Copy link
Member

No description provided.

@deribaucourt deribaucourt force-pushed the package-vsix branch 5 times, most recently from 45c68d1 to e7bf691 Compare October 27, 2023 14:19
@deribaucourt deribaucourt self-assigned this Oct 27, 2023
@deribaucourt deribaucourt marked this pull request as ready for review October 27, 2023 15:00
In order to package the client extension, it needs to have the server
compiled into it.
Updates all package.json. Some dependencies were missing when packaging
the client extension. Some were moved to the main package.json which
is responsible for compiling jest unit tests and integration tests.
With the previous updates to package.json, regenerate the lock files
with Node 20.
In order to package the client extension in a .vsix file, we need to
install the server node_modules into the client extension.
The .vscodeignore file was also updated to only contained the required
files at runtime.
This file must be present in the server directory for Jest tests.
In production, it must also be in the client directory.
Update the paths with the change from server/out to client/server for
all our typescript tools. Also update paths to ignore the .vsix
packaging artifacts.
Take into account the recent move from server/out to client/server.
These TODO are either done, or fell out of scope.
The .vsix can be installed in vscode. This commit checks that the
compilation works and archives the .vsix file as an artifact.
The badge will display the last pipeline result on the master branch.
So it won't be affected by current PR which might fail.
Only releases PR to master will affect the badge.
Copy link
Member

Choose a reason for hiding this comment

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

This file should not have been added. You can do installations and stuff from the root package: npm install stuff --workspace client. You can also update the package-lock.json file simply by calling "npm install" from the root package.

Copy link
Member Author

@deribaucourt deribaucourt Oct 30, 2023

Choose a reason for hiding this comment

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

Actually the client/ directory is the thing that gets packaged into the .vsix. So it needs to have client/node_modules as well as client/server/node_module. This follows the VSCode language server example packaging: https://github.com/microsoft/vscode-extension-samples/blob/main/lsp-sample/client/package-lock.json

},
"dependencies": {
"vscode-languageclient": "^8.1.0"
},
"devDependencies": {
"@types/node": "^20.6.2",
Copy link
Member

Choose a reason for hiding this comment

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

types are devDependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a new commit for that.

-p2222:22 \
-v "$(realpath ../..)":'/home/yoctouser/vscode-bitbake' \
--workdir '/home/yoctouser/vscode-bitbake/integration-tests/project-folder' \
-it vscode-test
Copy link
Member

Choose a reason for hiding this comment

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

should it be vscode-bitbake-test?

Otherwise I get the following error: Unable to find image 'vscode-test:latest' locally

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

"fetch": "npm run fetch:poky",
"fetch:poky": "mkdir -p resources/poky && curl -L -o resources/poky.tar.bz2 https://downloads.yoctoproject.org/releases/yocto/yocto-4.2.3/poky-aa63b25cbe25d89ab07ca11ee72c17cab68df8de.tar.bz2 && tar -xvjf resources/poky.tar.bz2 -C resources",
"compile": "tsc -b",
"compile": "tsc -b && npm run compile:server && npm run compile:client",
Copy link
Member

Choose a reason for hiding this comment

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

I think we should move at some point compile:server and compile:client to their respective packages.json so we can call all of them on one shot with npm run compile --workspaces. Especially with the "common" package coming soon and adding to it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had to remove the workspace property since it could not generate client/node_modules which is required by vsce.
The presented package.json architecture fulfills current needs. I suggest merging it that way and if new needs arise later, modify it.

Copy link
Member Author

Choose a reason for hiding this comment

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

The current package.json fulfills the packaging and testing needs we have while following the example documented example for VSCode language server extensions. I suggest merging this version and mofifying it when new needs arise.

Copy link
Member

Choose a reason for hiding this comment

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

same, it should not be there

Some dependencies were moved to devDependencies. I also made sure the
.vsix didn't contain the devDependencies.
@deribaucourt deribaucourt merged commit cf07d22 into staging Oct 30, 2023
1 check passed
@deribaucourt deribaucourt deleted the package-vsix branch October 30, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants