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

[plugin] Added extensionDependency automatically downloading. Fixes: #4504 #5379

Merged
merged 1 commit into from
Aug 18, 2019

Conversation

JPinkney
Copy link
Contributor

@JPinkney JPinkney commented Jun 6, 2019

This PR enables VSCode extension dependencies that comes from package.json.

You can test by using deploy by plugin id command with vscode:extension/vscjava.vscode-java-debug and vscode-java will automatically be installed before vscjava.vscode-java-debug is.

Fixes: #4504

Signed-off-by: Josh Pinkney joshpinkney@gmail.com

@JPinkney JPinkney changed the title Added extensionDependency automatically downloading. Fixes: #4504 [plugin] Added extensionDependency automatically downloading. Fixes: #4504 Jun 6, 2019
@akosyakov
Copy link
Member

akosyakov commented Jun 6, 2019

Should not it be a part of the vscode plugin resolver without any other changes? It could add a current extension, then look at it package.json, find dependencies, download them, unarchive them and then apply itself recursively till all dependencies are collected.

Otherwise it looks like a broken abstraction, since plugin system assumes that any dependency should be resolved as a vscode extension.

cc @evidolob @benoitf

@JPinkney
Copy link
Contributor Author

@akosyakov I've just fixed it up so that it almost entirely lives in the VSCode plugin resolver, if you don't mind reviewing again!

@JPinkney JPinkney force-pushed the addExtensionDependency branch 2 times, most recently from 0b6c8d9 to 1acc628 Compare June 18, 2019 18:24
@tsmaeder tsmaeder mentioned this pull request Jul 3, 2019
19 tasks
@akosyakov akosyakov added plug-in system issues related to the plug-in system vscode issues related to VSCode compatibility labels Jul 12, 2019
@tsmaeder
Copy link
Contributor

@benoitf @akosyakov what do we need to do to move this PR forward?

@tsmaeder tsmaeder mentioned this pull request Aug 14, 2019
24 tasks
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

Code looks good, i had some remarks please have a look.

@tsmaeder it should be rebased and add a notice which extension to use to test it. Someone can test after fixing remarks and rebasing.

packages/plugin-ext/src/common/plugin-protocol.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/main/node/plugin-deployer-impl.ts Outdated Show resolved Hide resolved
…heia#4504

Signed-off-by: Josh Pinkney <joshpinkney@gmail.com>
@JPinkney JPinkney requested a review from a team as a code owner August 15, 2019 18:50
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

Code wise it looks good to me and worked very well as well 👍

@JPinkney i will let it sit for today and merge tomorrow morning, just in case if @theia-ide/plugin-system want to check changes to the deployment logic

It would be good to nominate your as a committer as well! I hope @tsmaeder can do it, if not let me know i will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plug-in system issues related to the plug-in system vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There is no error when run VS code extensionthat use extensionDependencies that doesn't loaded
4 participants