-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[code] serve each webview from own origin #4738
Conversation
913ffc9
to
c8b3074
Compare
Codecov Report
@@ Coverage Diff @@
## main #4738 +/- ##
=========================================
+ Coverage 0 36.78% +36.78%
=========================================
Files 0 13 +13
Lines 0 3670 +3670
=========================================
+ Hits 0 1350 +1350
- Misses 0 2204 +2204
- Partials 0 116 +116
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
For the review I'd greatly benefit from a joint walkthrough. Would you have some time tomorrow? |
39b867c
to
ae57f7d
Compare
ae57f7d
to
ec8c304
Compare
Starting to review now |
/werft run 👍 started the job as gitpod-build-akosyakov-code-serve-each-webview-4529.16 |
@akosyakov Awesome change, really cool to see this "hack" resolved with a proper solution! 🚀 Plus the loom video really nails it: so much easier to grok the context and scope. I left a few comments, exclusively requests for more comments so my future me still knows what |
ec8c304
to
71a5d39
Compare
@geropl I refactored a bit and added comments with examples. Does it look good like that? I'm going to add |
71a5d39
to
f9006f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Tested and works as advertised.
f9006f3
to
ca7c3e2
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akosyakov, geropl The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ca7c3e2
to
7d0e10b
Compare
@csweichel I added workspace_cluster_host to the supervisor info endpoint to avoid parsing it from URL location. Could you have a look please? I thought to name it |
/lgtm |
decoupled from workpace origin (also extension host origin)
7d0e10b
to
74e60f0
Compare
New changes are detected. LGTM label has been removed. |
What it does
Change in Gitpod Code: gitpod-io/openvscode-server@6cf68ce
How to test
Bonus: start a workspace for github.com/gitpod-io/vscode, wait for the build and then check everything in dev version