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

Replace vscode with @types/vscode #246

Merged
merged 1 commit into from
Mar 22, 2020

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Mar 22, 2020

The main vscode dependency that we use has split up, and it's recommended to instead use @types/vscode and vscode-test. This pr replaces the vscode dependency with @types-vscode. I also update other various dependencies as a few of them were lagging behind which was causing some of the recent updates to not work right.

https://code.visualstudio.com/api/working-with-extensions/testing-extension#migrating-from-vscode

  • Update main vscode dependency
  • Update other various dependencies
  • Format code with new version of prettier

I did build locally and test installing the latest snapshot of metals, hovers, completions, debugging, and worksheets. Everything seems to be working as expected, but it's best if someone else also pulls this, builds it, and plays around with it, especially since I don't use vscode as my main editor. I'm probably the worst person to do this.

@gabro
Copy link
Member

gabro commented Mar 22, 2020

Make sure to thoroughly test this, by publishing a vsix locally and installing it by hand. I’ve tried this in the past and the extension would not activate (i wasn’t able to pinpoint the exact issue), although it was working fine when launching it from the debugger

Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

The relevant code changes are 👍 I’ve left a few minor comments.

However I find the prettier change a bit distracting and out of scope for the PR.
Can we do the prettier 2 upgrade in a separate PR so that we keep this diff on this one more focused?

package.json Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Also update other various dependencies
@ckipp01
Copy link
Member Author

ckipp01 commented Mar 22, 2020

Make sure to thoroughly test this, by publishing a vsix locally and installing it by hand.

Sounds good 👍 I was able to do this with no issues.

The relevant code changes are 👍 I’ve left a few minor comments.

Addressed the comments and made the changes.

However I find the prettier change a bit distracting and out of scope for the PR.
Can we do the prettier 2 upgrade in a separate PR so that we keep this diff on this one more focused?

Sure! I undid that update and I'll send in another pr for it.

@ckipp01 ckipp01 requested a review from gabro March 22, 2020 15:04
Copy link
Member

@gabro gabro left a comment

Choose a reason for hiding this comment

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

Thanks a lot of tackling this! It was long due :-) LGTM 👏

@gabro gabro merged commit 6ccdad1 into scalameta:master Mar 22, 2020
@ckipp01 ckipp01 deleted the dependency-updates branch March 22, 2020 15:14
kasiaMarek pushed a commit to kasiaMarek/metals-vscode that referenced this pull request Mar 29, 2023
…rn/types/node-14.14.32

Bump @types/node from 14.14.31 to 14.14.32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants