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

Bump API version to 1.83.1 #13118

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

rschnekenbu
Copy link
Contributor

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

@rschnekenbu
Copy link
Contributor Author

I tested with the 1.83.0 versions of the builtins-extension.
Testing done:

  • I did not notice any additional issue or error log.
  • editing of Typescript / json files was working,
  • the git views and actions I tested were also working.

tsmaeder
tsmaeder previously approved these changes Nov 29, 2023
Copy link
Contributor

@tsmaeder tsmaeder left a 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?

@tsmaeder
Copy link
Contributor

@rschnekenbu
Copy link
Contributor Author

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.
In our case, the version is compared when trying to install new extensions on open-vsix. The extensions provided on vsix may be compatible with 1.83.0 at the time they were released. If we set the API to 1.83.1, we loose these extensions in the view by default, even if they are supposed to be compatible. And I would not expect developers to update their extension compat version for any new patch release of vscode.
For the report you mention, we can basically set what we want in the parameters when generated, 1.83.0 or 1.83.1. I would not expect any differences in the generated report. Same as before, the API was compatible with 1.82.0, where the report indicates 1.82.3

@tsmaeder
Copy link
Contributor

If we set the API to 1.83.1, we loose these extensions in the view by default,

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.

@rschnekenbu
Copy link
Contributor Author

rschnekenbu commented Nov 29, 2023

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

getLatestCompatibleExtension(extensions: VSXExtensionRaw[]): VSXExtensionRaw | undefined;
)
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.

@tsmaeder
Copy link
Contributor

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?

@rschnekenbu
Copy link
Contributor Author

I suspect the code work as you say. I may have a check when I will have more time to do so.
Let's go with the 1.83.1 version then!

Contributed on behalf of ST Microelectronics

Signed-off-by: Remi Schnekenburger <[email protected]>
@rschnekenbu rschnekenbu changed the title Bump API version to 1.83.0 Bump API version to 1.83.1 Nov 29, 2023
@msujew
Copy link
Member

msujew commented Nov 29, 2023

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.

No, it doesn't do that. It's just an unintuitive argument ordering. This line:

return extensions.find(extension => this.versionGreaterThanOrEqualTo(extension.version, this.supportedApiVersion));

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 lte comparison on these two values:

return semver.lte(versionA, versionB);

@rschnekenbu
Copy link
Contributor Author

Thanks for clarification, @msujew!
That may need a small update of the naming and documentation then, as the doc says also "greater or Equal to"


I would be happy to handle this update if useful.

@msujew
Copy link
Member

msujew commented Nov 29, 2023

@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).

@rschnekenbu
Copy link
Contributor Author

I'll then create a follow-up task to investigate and clarify ;-)

@rschnekenbu
Copy link
Contributor Author

Issue created: #13123. Can one of you assign it to me?

@tsmaeder tsmaeder merged commit eeb1988 into eclipse-theia:master Nov 29, 2023
14 checks passed
@vince-fugnitto vince-fugnitto added this to the 1.44.0 milestone Nov 30, 2023
@rschnekenbu rschnekenbu deleted the issues/13065 branch December 4, 2023 09:44
@rschnekenbu rschnekenbu mentioned this pull request Jul 10, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[vscode] bump API compatibility version to 1.83.0
4 participants