Skip to content
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

Clean up and refactor VSCode integration #4010

Merged
merged 2 commits into from
Sep 30, 2021
Merged

Conversation

GirlBossRush
Copy link
Contributor

@GirlBossRush GirlBossRush commented Aug 19, 2021

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
@GirlBossRush GirlBossRush self-assigned this Aug 19, 2021
@github-actions
Copy link

github-actions bot commented Aug 19, 2021

✨ 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
@codecov
Copy link

codecov bot commented Aug 21, 2021

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
@GirlBossRush GirlBossRush force-pushed the 3835-lib-partial-upgrade branch from 82acf10 to 51abc84 Compare August 21, 2021 03:44
@GirlBossRush GirlBossRush force-pushed the 3835-lib-partial-upgrade branch from 51abc84 to 7580a52 Compare August 21, 2021 03:50
@GirlBossRush GirlBossRush force-pushed the 3835-lib-partial-upgrade branch from 7580a52 to aa34126 Compare August 21, 2021 03:58
@GirlBossRush GirlBossRush force-pushed the 3835-lib-partial-upgrade branch from cafb94c to 22fb477 Compare September 8, 2021 18:35
@jsjoeio jsjoeio modified the milestones: 3.12.0, 3.12.1 Sep 10, 2021
@jsjoeio
Copy link
Contributor

jsjoeio commented Sep 20, 2021

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

@benz0li
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?

@GirlBossRush GirlBossRush force-pushed the 3835-lib-partial-upgrade branch from 22fb477 to 2b49507 Compare September 30, 2021 03:16
@jsjoeio
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
Labels
enhancement Some improvement that isn't a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants