Skip to content

Fix progress reporting for the new LS - #4620

Merged
Mikhail Arkhipov (MikhailArkhipov) merged 3 commits into
microsoft:masterfrom
MikhailArkhipov:master
Mar 5, 2019
Merged

Mikhail Arkhipov (MikhailArkhipov) merged 3 commits into
microsoft:masterfrom
MikhailArkhipov:master

Conversation

@MikhailArkhipov

Copy link
Copy Markdown

Fix progress reporting for the new LS

  • Remove 'timeout' tracking since a) progress is not a good indicator of analysis completion and b) 60s may or may not be a timeout. LS is reporting its own telemetry anyway.

  • Make sure 'begin progress' is called even if there was no explicit notification. The problem is that onNotification cannot be attached to languageClient until it is ready - but LS may produce call to beginProgress while activator is still awaiting on the languageClient.ready and hence later reportProgress gets ignored.

  • 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!)

  • 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.

image

@DonJayamanne Don Jayamanne (DonJayamanne) added the no-changelog No news entry required label Mar 4, 2019
@codecov

codecov Bot commented Mar 4, 2019 •

Copy link
Copy Markdown

Codecov Report

Merging #4620 into master will increase coverage by 20%.
The diff coverage is 13%.

@@           Coverage Diff            @@
##           master   #4620     +/-   ##
========================================
+ Coverage      58%     77%    +20%     
========================================
  Files         369     445     +76     
  Lines       15942   21060   +5118     
  Branches     2506    3422    +916     
========================================
+ Hits         9135   16211   +7076     
+ Misses       6185    4843   -1342     
+ Partials      622       6    -616
Flag Coverage Δ
#Linux 65% <0%> (?)
#Windows 66% <0%> (?)
#macOS 65% <0%> (?)

@MikhailArkhipov
Mikhail Arkhipov (MikhailArkhipov) merged commit 7c255cd into microsoft:master Mar 5, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants