Remove roslyn implementation of error list #62741
Conversation
|
@dibarbet @davidwengier @mavasani @sharwell . Followup to #62735, but can be removed now (effectively almost all deletions). |
| // | ||
| // Temporary code. To keep tests running which test legacy behavior, we allow tests to configure this | ||
| // behavior. This code will be removed when we remove the legacy behavior entirely. | ||
| return GlobalOptions.GetOption(DiagnosticOptionsStorage.PullDiagnosticsFeatureFlag) ? NoOpIncrementalAnalyzer.Instance : analyzer; |
There was a problem hiding this comment.
Will be removed in followup.
| return; | ||
|
|
||
| await Task.Delay(TimeSpan.FromSeconds(1)); | ||
| } |
There was a problem hiding this comment.
no current way to ask lsp when it's done updating the UI. so polling for now.
There was a problem hiding this comment.
We have code in Razor that uses events and a semaphore to wait for errors in a slightly better way, if you're interested in future. I suspect neither is ideal though :)
|
Can #62735 be merged first to simplify the review here? |
Have smoke tested this and have communicated with CPS on their end
|
@davidwengier this is ready now. Thanks! |
| @@ -109,7 +109,7 @@ End Namespace | |||
| string.Join(Environment.NewLine, actualContents)); | |||
| } | |||
|
|
|||
| [IdeFact, WorkItem("https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1643350")] | |||
| [IdeFact(Skip = "https://github.com/dotnet/roslyn/issues/72428"), WorkItem("https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1643350")] | |||
There was a problem hiding this comment.
Is this test skipped because of the "tiny time slice" while things are loading? If so, seems it would be better to set the feature flag in the registry before running VS for the tests, to ensure not delay waiting for TN.
WIP. Can be merged in once we get enough confidence on LSP pull diagnostics.
Note: there would be a lot more cleanup after this. Lots of push-related diagnostic code needs to go away. I'm trying to figure out nice independent chunks that can be pulled out to make this manageable.
See also: #62735