-
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
Bump API version to 1.83.1 #13118
Bump API version to 1.83.1 #13118
Conversation
I tested with the 1.83.0 versions of the builtins-extension.
|
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.
Shouldn't this be 1.83.1?
I usually think that we are compatible with a major/minor version for an API, and the bug fixes versions are not supposed to interfere with any API compatibility. |
That's not how it works, IMO: any extensions that requires 1.83.0 will work with 1.83.1, but not the other way around. In particular, the built-ins for 1.83.1 (which is the release 1.83.x version) will not work with 1.83.0 per default. see https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/11676 We've got IP-approval for the 1.83.1 version. IMO, there is not api change between 1.83.0 and 1.83.1. |
Currently, osvx api filter compares the ranges of version for a given extension and looks for the one(s) who is/are greater than the supported API version. If the extension in the registry indicates 1.83.0, it won't be displayed, while still compatible with our code. (see
And the builtins version 1.83.1 will still be ok with our supported vscode version, and it would be indeed better to provide the 1.83.1 than the 1.83.0. |
Oh, wow! If I decided to release the 1.85.0 version of the built-ins, it would just pick that version, even though our api version is 1.83.1? That sounds like a bug! How does that work with non-theia clients of open-vsx? Anyway, the official VS Code version for the 1.83 release is 1.83.1: https://code.visualstudio.com/updates/v1_83 I still think it's the correct version to set. It would still work if I release the 1.83.1 built-ins, right? |
I suspect the code work as you say. I may have a check when I will have more time to do so. |
Contributed on behalf of ST Microelectronics Signed-off-by: Remi Schnekenburger <rschnekenburger@eclipsesource.com>
391e55b
to
fa33aba
Compare
No, it doesn't do that. It's just an unintuitive argument ordering. This line:
Ensures that our version of the supported VSCode API is greater or equal the version that is declared in the extension's package.json engine entry. We actually perform a
|
Thanks for clarification, @msujew!
I would be happy to handle this update if useful. |
@rschnekenbu Now I'm confused as well. That looks pretty weird. That code definitely needs a bit more clarification, even it is working correctly (which I'm not sure about). |
I'll then create a follow-up task to investigate and clarify ;-) |
Issue created: #13123. Can one of you assign it to me? |
What it does
Increase compatibility version to 1.83.0 as implementation tasks from task #13062 are closed.
Fixes #13065
Contributed on behalf of ST Microelectronics
How to test
Make sure the system starts and built-ins work as expected as described in https://github.com/eclipse-theia/vscode-builtin-extensions/wiki/Produce-and-Publish-the-set-of-vscode-built-in-extensions#local-testing
Review checklist
Reminder for reviewers