-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Spawn vscode on demand #4499
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
Spawn vscode on demand #4499
Conversation
✨ Coder.com for PR #4499 deployed! It will be updated on every commit.
- Host: https://codercom-1uhr6bd1m-codercom.vercel.app/docs/code-server
- Last deploy status: success
- Commit: f2b3156
- Workflow status: https://github.com/cdr/code-server/actions/runs/1460124342
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.
Thank you for doing this! I know it sucks to maintain legacy behavior. Once plugins are removed we can revert this behavior as well if we want. 😄
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.
Nice work!
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.
so much cleaner 👏
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.
👏
❓ why start the name with $? is that some sort of special pattern?
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.
It’s something of a convention I’ve seen help with readability, similarly to prefixing private methods with _. Prefixing a method like $proxyRequest helps indicate a unique purpose compared to a more lengthy proxyRequestHandler
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.
❓ What's the connectionToken used for again?
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.
Connection token is used by VS Code as a basic form of client-side security. At the moment we’ve disabled its usage, but I think it’s worth looking at again as upstream matures the feature.
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.
Related: #4479
One observation (this is not from this PR since I remember seeing this before) but I get a 404 when VS Code is still loading rather than the "VS Code may still be compiling" error (running yarn watch). For a new developer this could be unexpected.
b80725d to
e56d970
Compare
Codecov Report
@@ Coverage Diff @@ ## main #4499 +/- ## ========================================== - Coverage 66.46% 65.93% -0.53% ========================================== Files 30 30 Lines 1625 1638 +13 Branches 330 330 ========================================== Hits 1080 1080 - Misses 463 475 +12 - Partials 82 83 +1
Continue to review full report at Codecov.
|
* Fix : recreate the termux guide to adapt the recent changes Termux nodejs-lts changed from v14 to v16 and there are many issues people are facing such as with argon2. Hence I recommend changing it to this install process which is comparably better and has one less issue :^) I've also added some extra things such as installing GO and Python, idk about the TOC tree but this is pretty much it. * yarn-fmt and minor typos #4472 (comment) * Fix : replace unnecessary steps to be linked to a guide * Change from private gist to a section in Extra * Remove reference to non-existent step * ready to merge! Co-authored-by: Joe Previte <jjprevite@gmail.com>
Fixes #4481