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

Add dart sdk to full image and install dart and flutter extensions to next image #369

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

chrbayer
Copy link

@chrbayer chrbayer commented Jun 5, 2020

Please review. One drawback is that the dart SDK is installed to Dockerimage for stable and next image.

@chrbayer
Copy link
Author

chrbayer commented Jun 5, 2020

This fixes #368.

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.

Is there a reason that latest was not updated?

@chrbayer
Copy link
Author

chrbayer commented Jun 5, 2020

I have not added it to latest because it did not want to make this change for latest, the idea was to wait until next is getting latest. Do you think I should make the change in latest, too?

@vince-fugnitto
Copy link
Member

I have not added it to latest because it did not want to make this change for latest, the idea was to wait until next is getting latest. Do you think I should make the change in latest, too?

@chrbayer don’t see why we wouldn’t add the functionality to both images, they both should have the proper functionality to make use of the new vscode extensions introduced by the pull-request.

Please be sure to properly squash your commits when you’re ready for a re-review :)

@chrbayer
Copy link
Author

chrbayer commented Jun 5, 2020

Thanks for the review :-) I have added the plugins to the latest image, too. The pull request should be complete now.

But there was a problem with downloading repgrep, I had the same problem with ci and my local builds... This seem to work again.

But now it fails Error: Cannot find module '../lib/theia' on next image...
I had the same problem with this image before my changes, therefore I opened #371,
see eclipse-theia/theia#7974

@chrbayer
Copy link
Author

chrbayer commented Jun 8, 2020

This should be final now if nothing is found in the review.

@marcdumais-work
Copy link
Member

But there was a problem with downloading repgrep, I had the same problem with ci and my local builds... This seem to work again.

Indeed - the latest ripgrep was momentarily broken. It's now fixed. See microsoft/vscode-ripgrep#13

@marcdumais-work
Copy link
Member

Thanks @chrbayer . @vince-fugnitto ready for a final review on this one?

@chrbayer
Copy link
Author

chrbayer commented Jun 9, 2020

@marcdumais-work Ready from my side. Thanks in advance!

@vince-fugnitto
Copy link
Member

@chrbayer do you mind squashing your commits (into a single commit) so we can accept the changes? :)

@chrbayer
Copy link
Author

chrbayer commented Jun 9, 2020

@vince-fugnitto Of course, I will do it :-)

@marcdumais-work
Copy link
Member

@chrbayer I think the commit message needs an update to reflect the current content. I suggest something like:
[full] Add dart to latest and next images
or simply:
[full] Add dart

Signed-off-by: Christoph Bayer <chrbayer@criby.de>
@marcdumais-work
Copy link
Member

Awesome, thanks for the update @chrbayer

@vince-fugnitto Can you have another look, and if no further comments, merge?

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.

The changes look good, and the CI successfully passed 👍
Thank you for the contribution!

@vince-fugnitto vince-fugnitto merged commit 7deeca6 into theia-ide:master Jun 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants