vsx-registry: update api-compatibility handling#9280
Merged
vince-fugnitto merged 1 commit intomasterfrom Apr 9, 2021
Merged
Conversation
vince-fugnitto
added
builtins
vince-fugnitto
force-pushed
the
vf/vsx-updates
branch
from
0d0461e to
e135fab
Compare
vince-fugnitto
force-pushed
the
vf/vsx-updates
branch
from
e135fab to
a9483c3
Compare
Member
Author
|
@marechal-p I updated the pull-request based on the latest feedback 👍 |
paul-marechal
previously approved these changes
Apr 7, 2021
paul-marechal
dismissed
their stale review
Just noticed the this.api.getExtension(id) call if we are handling a builtin. Do we really need to make an API call when dealing with builtins?
vince-fugnitto
force-pushed
the
vf/vsx-updates
branch
5 times, most recently
from
2b07510 to
6774338
Compare
paul-marechal
approved these changes
Apr 7, 2021
The following commit updates the logic present in `@theia/vsx-registry` which is used to determine API-compatible extensions. The changes update the previous assumption that vscode-builtin extensions could not be downloaded through the extensions-view. The changes improve the performance overall when dealing with `builtin` extensions by not updating their information through the registry API calls which are unecessary. As a result, startup times are improved, local information such as the extension-editor is quicker, and there are less API-calls overall which do not burden the registry. The updated logic behaves in the following way: - builtins bundled with the application are excluded from fetching their compatible version. - builtins installed through the extensions-view are checked for their latest compatible version. Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
vince-fugnitto
force-pushed
the
vf/vsx-updates
branch
from
6774338 to
fbb0f54
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
The following pull-request updates the logic for determining which extensions from open-vsx are API-compatible.
The changes remove the previous assumption that vscode-builtin extensions could not be downloaded through the extensions-view.
The changes also improve the overall performance when dealing with builtins:
The changes include the following behavior updates:
package.json) behave like today, we skip determining their compatibility as they are controlled by application-developers (use the actual model withPluginType.Systemto determine builtins).How to test
package.jsonshould not be checked for their compatibility (as they are controlled by application developers).To verify step 1, you can do the following:
pluginsfolder -rm -rf plugins.(cd examples/electron && yarn start --vscode-api-version=1.29.0).typescript- thevscode-typescript-language-featuresshould not be present as there should be no version which supports this API level.Review checklist
Reminder for reviewers
Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com