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

Added current target version #18

Merged
merged 2 commits into from
Mar 21, 2022
Merged

Added current target version #18

merged 2 commits into from
Mar 21, 2022

Conversation

JonasHelming
Copy link
Contributor

@JonasHelming JonasHelming commented Mar 21, 2022

Add a new column to the compatibility report for a fixed VS Code version. This version is the current target reference version for compatibility, i.e. we aim to complete all missing API of this version.

fixed #17

Signed-off-by: Jonas Helming [email protected]

fixed #17

Signed-off-by: Jonas Helming <[email protected]>
Copy link

@jfaltermeier jfaltermeier left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm!

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I'm fine with the changes if we believe it has value, but I do have two suggestions:

  1. We should display our current coverage for the version we officially support (v1.53.2). I believe it is more meaningful then an arbitrary version we aim to target. We can have both, but 1.53.2 is likely more important.
  2. If we aim to support 1.55.2 (given you mentioned it is reasonably close to completion), then we can update our default API to this version, and rather than hardcode the value in this repo we simply read it from the main repo (means we won't need to update this repo).

@JonasHelming
Copy link
Contributor Author

I'm fine with the changes if we believe it has value, but I do have two suggestions:

  1. We should display our current coverage for the version we officially support (v1.53.2). I believe it is more meaningful then an arbitrary version we aim to target. We can have both, but 1.53.2 is likely more important.

makes perfect sense, I added it

  1. If we aim to support 1.55.2 (given you mentioned it is reasonably close to completion), then we can update our default API to this version, and rather than hardcode the value in this repo we simply read it from the main repo (means we won't need to update this repo).

I am not sure I understand this. If we raise the default API in the main repo, wouldn't we claim we support a version before we actually do?

@vince-fugnitto
Copy link
Member

I am not sure I understand this. If we raise the default API in the main repo, wouldn't we claim we support a version before we actually do?

We bump the API when we have a general idea that most things are supported. Given that we will soon have the new monaco, and you mentioned things are for the majority already supported we should already be thinking of bumping the API again. (There is no guarantee that everything in 1.53.2 is actually supported today either).

@JonasHelming
Copy link
Contributor Author

OK, but still I would claim that "target reference version" and "current reference version" are two different things. We could actually get the "currently reference version" from the main repo, not the target

@JonasHelming JonasHelming merged commit 7988404 into master Mar 21, 2022
@vince-fugnitto
Copy link
Member

OK, but still I would claim that "target reference version" and "current reference version" are two different things. We could actually get the "currently reference version" from the main repo, not the target

Correct, the current version should be pulled from the repo, the target should be something we decide as a community makes sense to aim for.

@JonasHelming
Copy link
Contributor Author

Do you have the location at hand where the version is define in the main repo? Otherwise I will look for it myself

@vince-fugnitto
Copy link
Member

Do you have the location at hand where the version is define in the main repo? Otherwise I will look for it myself

I linked it in #18 (comment):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add current reference target version
3 participants