Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

feat(typescript): upgrade to typescript 2.7.2 - #431

Merged
felixfbecker merged 1 commit into
sourcegraph:masterfrom
daviwil:typescript-2.7.2
Feb 16, 2018
Merged

feat(typescript): upgrade to typescript 2.7.2#431
felixfbecker merged 1 commit into
sourcegraph:masterfrom
daviwil:typescript-2.7.2

Conversation

@daviwil

@daviwil daviwil commented Feb 16, 2018

Copy link
Copy Markdown
Contributor

/cc @damieng

},
},
contents: [{ language: 'typescript', value: 'import Foo' }, '**alias**'],
contents: [{ language: 'typescript', value: 'class Foo\nimport Foo' }, '**alias**'],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like hovers now give you more symbol info than just the import statement!

@codecov

codecov Bot commented Feb 16, 2018

Copy link
Copy Markdown

Codecov Report

Merging #431 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #431     +/-   ##
=========================================
- Coverage   83.26%   83.16%   -0.1%     
=========================================
  Files          15       15             
  Lines        2038     2038             
  Branches      482      415     -67     
=========================================
- Hits         1697     1695      -2     
- Misses        339      341      +2     
  Partials        2        2
Impacted Files Coverage Δ
src/project-manager.ts 87.26% <0%> (-0.54%) ⬇️

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 2cac6cb...86f0842. Read the comment docs.

@daviwil

daviwil commented Feb 16, 2018

Copy link
Copy Markdown
Contributor Author

Anything I can/should do about the drop in code coverage?

@felixfbecker

Copy link
Copy Markdown
Contributor

Hmm, seems like getNewLine() is not called anymore by TS. Could you check if it is still part of the interface? If not, we can safely remove it

https://codecov.io/gh/sourcegraph/javascript-typescript-langserver/pull/431/changes#L95

@daviwil

daviwil commented Feb 16, 2018

Copy link
Copy Markdown
Contributor Author

Still part of the interface: https://github.com/Microsoft/TypeScript/blob/v2.7.2/src/services/types.ts#L163

Looks like there was a recent change to prioritize the use of FormatCodeSettings if it's provided before ever calling getNewLine(), so that could account for the lack of calls:

https://github.com/Microsoft/TypeScript/pull/21158/files#diff-183e2291cbb94d65ae0406354ce1ba7aR1262

I'll remove that function from the language service host and see what happens!

@felixfbecker

Copy link
Copy Markdown
Contributor

In that case just leave it in and not worry about the coverage

@daviwil

daviwil commented Feb 16, 2018

Copy link
Copy Markdown
Contributor Author

Ok, sounds good!

@felixfbecker
felixfbecker merged commit 8a87797 into sourcegraph:master Feb 16, 2018
@daviwil
daviwil deleted the typescript-2.7.2 branch February 16, 2018 22:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants