-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
prefer listed plugins to dependencies #6038
Conversation
|
||
// now that we have plugins check if we have File Handler for them | ||
await this.applyFileHandlers(pluginDeployerEntries); | ||
|
||
// ok now ask for directory handlers | ||
await this.applyDirectoryFileHandlers(pluginDeployerEntries); | ||
|
||
// add current plugin deployer entries first because dependencies to be installed first |
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.
@JPinkney It does not matter in which order plugins are deployed. They are only loaded when a frontend is connected by then all plugins should be already on the disk.
@vince-fugnitto @marcdumais-work it is important for 0.10.0 |
otherwise the wrong version gets installed from a marketplace instead of from local vsix file for example Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
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 it both the way you listed and by using deploy by plugin id
with the ms-kubernetes-tools.vscode-kubernetes-tools
extension (it has vscode-yaml as extension dependency) and both seem to work
@AlexTugarev could you please approve it as well @JPinkney is not an official committer yet, unfortunately, PR can be only merged after the approved by a committer |
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.
The code changes look good to me! 🙏
What it does
It's a bug fix for #5379. We should either test and merge this PR for 0.10.0, or revert #5379. Right now it breaks all products which consumes from local files.
How to test
plugins
instead of vsixnpm install
npm run watch
to compileReview checklist
Reminder for reviewers