Skip to content

Allow LS to be bundled, versioned - #12034

Merged
Jake Bailey (jakebailey) merged 11 commits into
microsoft:masterfrom
jakebailey:ls-bundle
Jun 1, 2020
Merged

Allow LS to be bundled, versioned#12034
Jake Bailey (jakebailey) merged 11 commits into
microsoft:masterfrom
jakebailey:ls-bundle

Conversation

@jakebailey

Copy link
Copy Markdown
Member

Allows the download to be skipped and a pinned version to be used instead.

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • The wiki is updated with any design decisions/details.

@jakebailey Jake Bailey (jakebailey) added the no-changelog No news entry required label May 28, 2020
Comment thread src/client/activation/node/languageServerFolderService.ts
Comment thread src/client/activation/node/languageServerFolderService.ts Outdated
Comment thread src/client/activation/common/downloader.ts Outdated
@MikhailArkhipov

Copy link
Copy Markdown

LGTM. Eric Snow (@ericsnowcurrently) may want to approve.

@jakebailey

Copy link
Copy Markdown
Member Author

Actually, one thing I may want to do is rename the folder so that it's not the same as the local copy that gets run. That way there's no ambiguity as to where it comes from if we are doing local development. Don't merge this quite yet, want to see if that turns out to be better...

Comment thread src/client/activation/node/languageServerFolderService.ts
Comment thread src/client/activation/types.ts

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mostly LGTM

I've left a few comments, mostly for clarificaiton and some very minor changes.

Comment thread src/test/activation/languageServer/downloader.unit.test.ts Outdated
Comment thread package.json Outdated
Comment thread src/client/activation/node/languageServerFolderService.ts
Comment thread src/client/activation/node/languageServerFolderService.ts
Comment thread src/client/activation/node/languageServerFolderService.ts Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud

sonarqubecloud Bot commented Jun 1, 2020

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jakebailey
Jake Bailey (jakebailey) merged commit e42fc1e into microsoft:master Jun 1, 2020
@jakebailey
Jake Bailey (jakebailey) deleted the ls-bundle branch June 1, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants