Skip to content

Conversation

sgraband
Copy link
Contributor

@sgraband sgraband commented Jul 2, 2025

What it does

This is needed because otherwise executing something on the container will fail. Also make sure to report the error channel instead of the stdout channel if starting the container fails. This will require a recompiling of the native dependencies.

How to test

  1. Start a devcontainer and see that it now works.

On master, starting a devcontainer will not work and fail with Could not start remote system:.

Follow-ups

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

Reminder for reviewers

This is needed because otherwise executing something on the container will fail.
Also make sure to report the error channel instead of the stdout channel if starting the container fails.
This will require a recompiling of the native dependencies.

Signed-off-by: Simon Graband <sgraband@eclipsesource.com>
@sgraband sgraband requested a review from ndoschek July 2, 2025 12:33
@github-project-automation github-project-automation bot moved this to Waiting on reviewers in PR Backlog Jul 2, 2025
Copy link
Contributor

@ndoschek ndoschek left a comment

Choose a reason for hiding this comment

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

Thanks @sgraband for this fix!
I tested the glsp-website-source repository to open in the Dev Container and it works again as expected! 👍

@ndoschek ndoschek requested a review from rschnekenbu July 2, 2025 15:12
Copy link
Contributor

@rschnekenbu rschnekenbu left a comment

Choose a reason for hiding this comment

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

LGTM! I only looked at the code, feature was tested by @ndoschek

@github-project-automation github-project-automation bot moved this from Waiting on reviewers to Needs merge in PR Backlog Jul 2, 2025
@rschnekenbu rschnekenbu merged commit 9a427a0 into master Jul 2, 2025
10 of 12 checks passed
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Jul 2, 2025
@github-actions github-actions bot added this to the 1.64.0 milestone Jul 2, 2025
@sgraband sgraband mentioned this pull request Jul 3, 2025
1 task
sgraband added a commit that referenced this pull request Jul 7, 2025
This is needed because otherwise executing something on the container will fail.
Also make sure to report the error channel instead of the stdout channel if starting the container fails.
This will require a recompiling of the native dependencies.

Signed-off-by: Simon Graband <sgraband@eclipsesource.com>
laemmleint pushed a commit to mvtecsoftware/theia that referenced this pull request Aug 18, 2025
This is needed because otherwise executing something on the container will fail.
Also make sure to report the error channel instead of the stdout channel if starting the container fails.
This will require a recompiling of the native dependencies.

Signed-off-by: Simon Graband <sgraband@eclipsesource.com>
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.

3 participants