Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Clean up and refactor VSCode integration #4010

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

Merged
GirlBossRush merged 2 commits into main from 3835-lib-partial-upgrade
Sep 30, 2021
Merged

Conversation

Copy link
Contributor

@GirlBossRush GirlBossRush commented Aug 19, 2021 •
edited
Loading

This PR addresses portions of #3835, specifically changes associated with making our modifications to upstream more aligned with their coding architecture and style. To keep this PR reviewable, a second PR based off the remaining changes in our fork branch will follow.

  • Fixed linter/prettier mixups during development.
  • Cleaned up web config injection, removed separate NLS injection.
  • Cleaned up comments for use with intellisense
  • Updated types to more often use VScode's built in types.
  • Cleaned up URI helpers.
  • Organized connection classes into separate files.
  • Documented protocol usage, adding trace logging support.
  • Fleshed out partial support for remote debugging.
  • Updated tests.

How to review this PR

Despite the file count, much of the changes here are linter corrections, removing duplicate interfaces, and moving existing interfaces into better documented areas. I recommend reviewing with whitespace changes disabled.

@GirlBossRush GirlBossRush added the enhancement Some improvement that isn't a feature label Aug 19, 2021
Copy link

github-actions bot commented Aug 19, 2021 •
edited
Loading

✨ Coder.com for PR #4010 deployed! It will be updated on every commit.

@GirlBossRush GirlBossRush force-pushed the 3835-lib-partial-upgrade branch 10 times, most recently from 9883098 to 82acf10 Compare August 21, 2021 03:33
@GirlBossRush GirlBossRush changed the title (削除) VScode fork, partial upgrade (削除ここまで) (追記) Clean up and refactor VSCode integration (追記ここまで) Aug 21, 2021
@GirlBossRush GirlBossRush added this to the 3.12.0 milestone Aug 21, 2021
Copy link

codecov bot commented Aug 21, 2021 •
edited
Loading

Codecov Report

Merging #4010 (ea1a59d) into main (6eda7ae) will decrease coverage by 65.32%.
The diff coverage is n/a.

❗ Current head ea1a59d differs from pull request most recent head 9f4e932. Consider uploading reports for the commit 9f4e932 to get more accurate results
Impacted file tree graph 

@@ Coverage Diff @@
## main #4010 +/- ##
==========================================
- Coverage 65.32% 0 -65.33% 
==========================================
 Files 36 0 -36 
 Lines 1883 0 -1883 
 Branches 380 0 -380 
==========================================
- Hits 1230 0 -1230 
+ Misses 555 0 -555 
+ Partials 98 0 -98
Impacted Files Coverage Δ
src/node/routes/pathProxy.ts
src/node/routes/update.ts
src/common/http.ts
src/node/routes/health.ts
src/node/proxy.ts
src/node/routes/apps.ts
src/node/routes/domainProxy.ts
src/node/coder_cloud.ts
src/node/heart.ts
src/node/wsRouter.ts
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6eda7ae...9f4e932. Read the comment docs.

@GirlBossRush GirlBossRush marked this pull request as ready for review August 21, 2021 03:40
@GirlBossRush GirlBossRush requested a review from a team as a code owner August 21, 2021 03:40
Copy link
Contributor

jsjoeio commented Sep 20, 2021

@TeffenEllis are you planning to update this, this week?

Copy link
Contributor

benz0li commented Sep 29, 2021

@TeffenEllis Is the VS Code integration refactored in the manner of https://github.com/gitpod-io/openvscode-server?

Copy link
Contributor

jsjoeio commented Sep 30, 2021

Looks like yarn fmt is unhappy with you 😛

image

Copy link
Contributor

@jsjoeio jsjoeio left a comment

Choose a reason for hiding this comment

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

Merging #4010 (8a295c2) into main (6eda7ae) will decrease coverage by 1.07%.

@GirlBossRush GirlBossRush force-pushed the 3835-lib-partial-upgrade branch 6 times, most recently from ea1a59d to 9f4e932 Compare September 30, 2021 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kylecarbs kylecarbs kylecarbs left review comments

@code-asher code-asher code-asher left review comments

+1 more reviewer

@jsjoeio jsjoeio jsjoeio approved these changes

Reviewers whose approvals may not affect merge requirements
Labels
enhancement Some improvement that isn't a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /