Skip to content
This repository has been archived by the owner on Oct 5, 2022. It is now read-only.

add vsx-registry to ci builds #330

Merged
merged 5 commits into from
Apr 13, 2020
Merged

add vsx-registry to ci builds #330

merged 5 commits into from
Apr 13, 2020

Conversation

emdfonseca
Copy link
Contributor

@emdfonseca emdfonseca commented Apr 3, 2020

What it does

Add @theia/vsx-registry dependency to latest and next package.json files of every app that is part of the CI pipeline (theia-cpp-docker theia-dart-docker theia-docker theia-full-docker theia-go-docker theia-java-docker theia-python-docker theia-rust-docker theia-swift-docker).

Closes #325

How to test

Trigger a build and let it finish. Everything builds without any issue.

Review checklist

Reminder for reviewers

Signed-off-by: Emanuel Fonseca emdfonseca@gmail.com

Closes #325

Signed-off-by: Emanuel Fonseca <emdfonseca@gmail.com>
Signed-off-by: Emanuel Fonseca <emdfonseca@gmail.com>
@emdfonseca emdfonseca marked this pull request as ready for review April 10, 2020 09:00
- updated rust channel to nightly-2020-04-10

Signed-off-by: Emanuel Fonseca <emdfonseca@gmail.com>
@emdfonseca
Copy link
Contributor Author

I updated the rust channel to "nightly-2020-04-10" in order to build successfully. If this needs to move out of the PR just let me know.

@emdfonseca emdfonseca closed this Apr 10, 2020
@emdfonseca emdfonseca reopened this Apr 10, 2020
@emdfonseca emdfonseca closed this Apr 10, 2020
@emdfonseca emdfonseca reopened this Apr 10, 2020
Signed-off-by: Emanuel Fonseca <emdfonseca@gmail.com>
@emdfonseca
Copy link
Contributor Author

Build tests frequently timeout on theia-rust. Build is running fine locally. Should we first merge #323 before approving this one?

@marcdumais-work
Copy link
Member

Build tests frequently timeout on theia-rust. Build is running fine locally. Should we first merge #323 before approving this one?

done

…dd-vsx-registry

Signed-off-by: Emanuel Fonseca <emdfonseca@gmail.com>
@@ -41,6 +41,7 @@
"@theia/userstorage": "next",
"@theia/variable-resolver": "next",
"@theia/workspace": "next",
"@theia/vsx-registry": "next",
Copy link
Member

Choose a reason for hiding this comment

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

It seems low-risk to keep this change in this untested image, so I am ok to leave this as-is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I built the rust image locally without any issues so the vsx-registry does not cause any issues. I guess the problem on travis was related to system performance which was causing the tests to take longer than 30s. I didn't investigate into this issue though.

Copy link
Member

Choose a reason for hiding this comment

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

I built the rust image locally without any issues so the vsx-registry does not cause any issues. I guess the problem on travis was related to system performance which was causing the tests to take longer than 30s. I didn't investigate into this issue though.

I think I found the issue: we are still including a deprecated extension for this image: @theia/editorconfig. Long story short, this causes the app never to start, which eventually fails CI.

I'm removing this extension in this WIP PR: #343

@marcdumais-work
Copy link
Member

@emdfonseca Thanks for the updates. Looks good to me.

The only one I am not sure is theia-rust: since it's now out of CI, one option is not to touch it in this PR. OTOH we hope to be able to fix CI it ASAP, so it may not hurt to include the vsx-registry extension even though it's not in CI ATM. WDYT?

@emdfonseca emdfonseca closed this Apr 13, 2020
@emdfonseca emdfonseca reopened this Apr 13, 2020
@emdfonseca
Copy link
Contributor Author

I'd accept this one cause it already adds value to the theia-apps repo without any issues. We can bring rust image back again later.

@emdfonseca
Copy link
Contributor Author

I triggered the build again just because the status of the travis build was not updated in the PR. Is this normal or did I do something wrong?

Copy link
Member

@marcdumais-work marcdumais-work left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @emdfonseca

@marcdumais-work
Copy link
Member

I triggered the build again just because the status of the travis build was not updated in the PR. Is this normal or did I do something wrong?

I noticed that earlier too - I do not see that it's related to your PR.

@marcdumais-work marcdumais-work merged commit c294cb1 into theia-ide:master Apr 13, 2020
@marcdumais-work
Copy link
Member

I should probably have asked for the commits to be squashed into one, but too late now :)

@marcdumais-work
Copy link
Member

@emdfonseca if you'd like to have a look at this follow-up PR . It builds-up from your update of the rust channel and fixes a couple of other things so that the image starts and vsx registry can be used from it.

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

Successfully merging this pull request may close these issues.

include @theia/vsx-registry extension in images to allow installing extensions from Open VSX Registry
2 participants