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

Contribute a vscode-extensions.json file #499

Merged
merged 4 commits into from
Jul 3, 2020
Merged

Contribute a vscode-extensions.json file #499

merged 4 commits into from
Jul 3, 2020

Conversation

ericwill
Copy link
Contributor

@ericwill ericwill commented Jun 15, 2020

This file lists all extensions in the che-plugin-registry. By extension, I mean those Che plugins that use VS Code extensions.

The fields are basic (for now) but are as follows:

  • revision: the tag or SHA1 ID of the upstream repository, capturing a certain version of the extension
  • vsixDirectory (optional): the sub-directory where the extension is located (some repositories have multiple extensions in multiple folders)
  • repository: the location of the upstream repository

This file is needed for eclipse-che/che#17014, which is part of a greater epic eclipse-che/che#15819.

Signed-off-by: Eric Williams ericwill@redhat.com

@che-bot
Copy link
Contributor

che-bot commented Jun 15, 2020

Happy path tests passed.

@ericwill ericwill requested a review from l0rd June 16, 2020 03:29
vscode-extensions.json Outdated Show resolved Hide resolved
vscode-extensions.json Outdated Show resolved Hide resolved
@che-bot
Copy link
Contributor

che-bot commented Jun 18, 2020

Happy path tests passed.

This file lists all extensions in the che-plugin-registry. By extension,
I mean those Che plugins that use VS Code Extensions.

The fields are basic (for now) but are as follows:
* version: the version of the Che plugin in the registry
* checkout: the checkout tag needed to checkout the VS Code Extension in its upstream repository
* vsixDir: the sub-directory where the extension is located (some repositories have multiple extensions in multiple folders)
* repository: the location of the upstream repository

This file is needed for eclipse-che/che#17014, which is part of a greater epic eclipse-che/che#15819.

Signed-off-by: Eric Williams <ericwill@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Jun 29, 2020

Happy path tests passed.

@l0rd
Copy link
Contributor

l0rd commented Jun 30, 2020

Isn't the version already available in package.json files?
Can we avoid using branches as checkouts (e.g. master) but use only tags or sha1 to allow reproducible builds?

@ericwill
Copy link
Contributor Author

Isn't the version already available in package.json files?

Yes, but in this case the version is what we have in the registry, not what is available upstream. We compare this version to what's in the package.json.

Can we avoid using branches as checkouts (e.g. master) but use only tags or sha1 to allow reproducible builds?

Sure, the majority of these use tags to begin with. I will update the branch cases to use SHA1 IDs.

@l0rd
Copy link
Contributor

l0rd commented Jun 30, 2020

Yes, but in this case the version is what we have in the registry, not what is available upstream. We compare this version to what's in the package.json.

Can we just set the versions in our registry to match the versions of the package.json? Having to maintain versions both in the package.json and the vscode-extension.json looks like an avoidable overhead.

@ericwill
Copy link
Contributor Author

Yes, but in this case the version is what we have in the registry, not what is available upstream. We compare this version to what's in the package.json.

Can we just set the versions in our registry to match the versions of the package.json? Having to maintain versions both in the package.json and the vscode-extension.json looks like an avoidable overhead.

We keep track of the version we have in the registry so that when we upgrade it (someone opens a PR on vscode-extensions.json), we can run tests on the new version of the plugin and make sure it works before merging it. We can't just use the upstream version due to API incompatibilities, potential changes to the sidecars, etc. These need to be verified before we can adopt a new version of the plugin in the registry.

Signed-off-by: Eric Williams <ericwill@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Jun 30, 2020

Happy path tests passed.

Remove version, rename checkout -> revision, update SHA1 ID's, and bump plugin versions
that have since changed.

Signed-off-by: Eric Williams <ericwill@redhat.com>
@ericwill
Copy link
Contributor Author

ericwill commented Jul 2, 2020

@l0rd I've amended the file based on our discussions this morning, please review.

@che-bot
Copy link
Contributor

che-bot commented Jul 2, 2020

Happy path tests passed.

Copy link
Contributor

@l0rd l0rd left a comment

Choose a reason for hiding this comment

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

LGTM. Please update the PR description that still has the old fields and add a section in the README.md that explains what's the file for (now and in the long term), what are the available fields right now and provide an example.

@benoitf
Copy link
Contributor

benoitf commented Jul 3, 2020

probably vsixDirectory could be simplified/just named directory as we target VS Code extensions (vsix) in any case

side note: could it be sorted and use repository as first entry ?

Also there is one thing which is not clear to me:
do we retrieve the vsix from the release page or we only build from sources ?
if we build from source, probably we need to know if npm or yarn needs to be used (or it's assuming based on yarn.lock or package-lock.json)

@ericwill
Copy link
Contributor Author

ericwill commented Jul 3, 2020

probably vsixDirectory could be simplified/just named directory as we target VS Code extensions (vsix) in any case

side note: could it be sorted and use repository as first entry ?

Sure I can do that.

Also there is one thing which is not clear to me:
do we retrieve the vsix from the release page or we only build from sources ?
if we build from source, probably we need to know if npm or yarn needs to be used (or it's assuming based on yarn.lock or package-lock.json)

It hasn't been decided yet, so the fields are missing until we land on a way forward. Right now this is the basic version of the file, so we can get the automated report generation working. I can revise this file to add the necessary fields in another PR once we get to the build/publish steps.

Signed-off-by: Eric Williams <ericwill@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Jul 3, 2020

@ericwill Happy Path PR check [build 138] failed.

Re-trigger by [ci-test] or [ci-test-happy-path].

Link URL
console https://ci.centos.org/job/devtools-che-plugin-registry-pr-check-happy-path/138/console
artifacts http://artifacts.ci.centos.org/devtools/che/che-plugin-registry-prcheck/138/

Depending on failure reason, the artifacts or deployment may not be present.

@ericwill ericwill merged commit c640416 into eclipse-che:master Jul 3, 2020
@l0rd l0rd added the new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes label Jul 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants